diff mbox

[U-Boot,v2,1/3] Add limits.h to hold basic limits

Message ID 1318274551-25335-2-git-send-email-sjg@chromium.org
State New, archived
Headers show

Commit Message

Simon Glass Oct. 10, 2011, 7:22 p.m. UTC
This brings a basic limits.h implementation into U-Boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 fs/ubifs/ubifs.h |    4 +---
 include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)
 create mode 100644 include/limits.h

Comments

Albert ARIBAUD Oct. 21, 2011, 7:54 p.m. UTC | #1
Hi Simon,

Le 10/10/2011 21:22, Simon Glass a écrit :
> This brings a basic limits.h implementation into U-Boot.
>
> Signed-off-by: Simon Glass<sjg@chromium.org>
> ---
>   fs/ubifs/ubifs.h |    4 +---
>   include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+), 3 deletions(-)
>   create mode 100644 include/limits.h

Apparently, in all the U-Boot codebase, only one file required standard 
limit names, and gets them with three lines of code. Is it worth adding 
40 lines of code for this?

Amicalement,
Simon Glass Oct. 21, 2011, 8:19 p.m. UTC | #2
Hi Albert,

On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
> Le 10/10/2011 21:22, Simon Glass a écrit :
>>
>> This brings a basic limits.h implementation into U-Boot.
>>
>> Signed-off-by: Simon Glass<sjg@chromium.org>
>> ---
>>  fs/ubifs/ubifs.h |    4 +---
>>  include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 41 insertions(+), 3 deletions(-)
>>  create mode 100644 include/limits.h
>
> Apparently, in all the U-Boot codebase, only one file required standard
> limit names, and gets them with three lines of code. Is it worth adding 40
> lines of code for this?

Well 2/3 is the license header - and I thought it best to add all the
limits in one go. I can add just those few if you like.

This file is used later in the patch series.

Regards,
Simon

>
> Amicalement,
> --
> Albert.
>
Albert ARIBAUD Oct. 21, 2011, 9 p.m. UTC | #3
Le 21/10/2011 22:19, Simon Glass a écrit :
> Hi Albert,
>
> On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Hi Simon,
>>
>> Le 10/10/2011 21:22, Simon Glass a écrit :
>>>
>>> This brings a basic limits.h implementation into U-Boot.
>>>
>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>> ---
>>>   fs/ubifs/ubifs.h |    4 +---
>>>   include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 41 insertions(+), 3 deletions(-)
>>>   create mode 100644 include/limits.h
>>
>> Apparently, in all the U-Boot codebase, only one file required standard
>> limit names, and gets them with three lines of code. Is it worth adding 40
>> lines of code for this?
>
> Well 2/3 is the license header - and I thought it best to add all the
> limits in one go. I can add just those few if you like.
>
> This file is used later in the patch series.

I don't see much use of these in the subsequent patches either -- and 
those few uses could be discussed, such as for instance the use of 
INT_MAX as the maximum buffer size for some *printf() functions -- 
actually, anything very big would fit just as well, would it not?

> Regards,
> Simon

Amicalement,
Simon Glass Oct. 21, 2011, 9:12 p.m. UTC | #4
Hi Albert,

On Fri, Oct 21, 2011 at 2:00 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 21/10/2011 22:19, Simon Glass a écrit :
>>
>> Hi Albert,
>>
>> On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net>  wrote:
>>>
>>> Hi Simon,
>>>
>>> Le 10/10/2011 21:22, Simon Glass a écrit :
>>>>
>>>> This brings a basic limits.h implementation into U-Boot.
>>>>
>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>> ---
>>>>  fs/ubifs/ubifs.h |    4 +---
>>>>  include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 41 insertions(+), 3 deletions(-)
>>>>  create mode 100644 include/limits.h
>>>
>>> Apparently, in all the U-Boot codebase, only one file required standard
>>> limit names, and gets them with three lines of code. Is it worth adding
>>> 40
>>> lines of code for this?
>>
>> Well 2/3 is the license header - and I thought it best to add all the
>> limits in one go. I can add just those few if you like.
>>
>> This file is used later in the patch series.
>
> I don't see much use of these in the subsequent patches either -- and those
> few uses could be discussed, such as for instance the use of INT_MAX as the
> maximum buffer size for some *printf() functions -- actually, anything very
> big would fit just as well, would it not?

Yes it would, it's doesn't really need to be INT_MAX. Then again,
limits.h is a fairly standard file to have around, and INT_MAX is an
efficient 'large' value to load on many architectures.

In any case it seems wrong to me that ubifs is defining its own
INT_MAX and U-Boot doesn't have one.

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>
Albert ARIBAUD Oct. 21, 2011, 9:47 p.m. UTC | #5
Le 21/10/2011 23:12, Simon Glass a écrit :
> Hi Albert,
>
> On Fri, Oct 21, 2011 at 2:00 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 21/10/2011 22:19, Simon Glass a écrit :
>>>
>>> Hi Albert,
>>>
>>> On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
>>> <albert.u.boot@aribaud.net>    wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> Le 10/10/2011 21:22, Simon Glass a écrit :
>>>>>
>>>>> This brings a basic limits.h implementation into U-Boot.
>>>>>
>>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>>> ---
>>>>>   fs/ubifs/ubifs.h |    4 +---
>>>>>   include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>>>>   2 files changed, 41 insertions(+), 3 deletions(-)
>>>>>   create mode 100644 include/limits.h
>>>>
>>>> Apparently, in all the U-Boot codebase, only one file required standard
>>>> limit names, and gets them with three lines of code. Is it worth adding
>>>> 40
>>>> lines of code for this?
>>>
>>> Well 2/3 is the license header - and I thought it best to add all the
>>> limits in one go. I can add just those few if you like.
>>>
>>> This file is used later in the patch series.
>>
>> I don't see much use of these in the subsequent patches either -- and those
>> few uses could be discussed, such as for instance the use of INT_MAX as the
>> maximum buffer size for some *printf() functions -- actually, anything very
>> big would fit just as well, would it not?
>
> Yes it would, it's doesn't really need to be INT_MAX. Then again,
> limits.h is a fairly standard file to have around, and INT_MAX is an
> efficient 'large' value to load on many architectures.
>
> In any case it seems wrong to me that ubifs is defining its own
> INT_MAX and U-Boot doesn't have one.

My point is precisely that as standard as limits.h is, U-Boot has 
managed to survive not having it around so far, which kind of shows 
limits.h is not needed.

> Regards,
> Simon

Amicalement,
Simon Glass Oct. 21, 2011, 10:02 p.m. UTC | #6
Hi Albert,

On Fri, Oct 21, 2011 at 2:47 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 21/10/2011 23:12, Simon Glass a écrit :
>>
>> Hi Albert,
>>
>> On Fri, Oct 21, 2011 at 2:00 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net>  wrote:
>>>
>>> Le 21/10/2011 22:19, Simon Glass a écrit :
>>>>
>>>> Hi Albert,
>>>>
>>>> On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
>>>> <albert.u.boot@aribaud.net>    wrote:
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>> Le 10/10/2011 21:22, Simon Glass a écrit :
>>>>>>
>>>>>> This brings a basic limits.h implementation into U-Boot.
>>>>>>
>>>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>>>> ---
>>>>>>  fs/ubifs/ubifs.h |    4 +---
>>>>>>  include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>>>>>  2 files changed, 41 insertions(+), 3 deletions(-)
>>>>>>  create mode 100644 include/limits.h
>>>>>
>>>>> Apparently, in all the U-Boot codebase, only one file required standard
>>>>> limit names, and gets them with three lines of code. Is it worth adding
>>>>> 40
>>>>> lines of code for this?
>>>>
>>>> Well 2/3 is the license header - and I thought it best to add all the
>>>> limits in one go. I can add just those few if you like.
>>>>
>>>> This file is used later in the patch series.
>>>
>>> I don't see much use of these in the subsequent patches either -- and
>>> those
>>> few uses could be discussed, such as for instance the use of INT_MAX as
>>> the
>>> maximum buffer size for some *printf() functions -- actually, anything
>>> very
>>> big would fit just as well, would it not?
>>
>> Yes it would, it's doesn't really need to be INT_MAX. Then again,
>> limits.h is a fairly standard file to have around, and INT_MAX is an
>> efficient 'large' value to load on many architectures.
>>
>> In any case it seems wrong to me that ubifs is defining its own
>> INT_MAX and U-Boot doesn't have one.
>
> My point is precisely that as standard as limits.h is, U-Boot has managed to
> survive not having it around so far, which kind of shows limits.h is not
> needed.

By that logic one would never do any code clean ups. Do I understand
you correctly?

But this is the least of my concerns :-) If you don't want it, don't
take it. Shall I modify the series to define its own INT_MAX, or just
chose a large number?

BTW I think you are looking at the old version of that patch series -
we are now on v4. The limits.h patch is the same though. Later on in
the series I add comments to vsprintf() functions and move them into
their own header. If you apply the same logic there then that tidy is
not needed also. Please let me know.

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>
Albert ARIBAUD Oct. 21, 2011, 10:39 p.m. UTC | #7
Le 22/10/2011 00:02, Simon Glass a écrit :
> Hi Albert,
>
> On Fri, Oct 21, 2011 at 2:47 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 21/10/2011 23:12, Simon Glass a écrit :
>>>
>>> Hi Albert,
>>>
>>> On Fri, Oct 21, 2011 at 2:00 PM, Albert ARIBAUD
>>> <albert.u.boot@aribaud.net>    wrote:
>>>>
>>>> Le 21/10/2011 22:19, Simon Glass a écrit :
>>>>>
>>>>> Hi Albert,
>>>>>
>>>>> On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
>>>>> <albert.u.boot@aribaud.net>      wrote:
>>>>>>
>>>>>> Hi Simon,
>>>>>>
>>>>>> Le 10/10/2011 21:22, Simon Glass a écrit :
>>>>>>>
>>>>>>> This brings a basic limits.h implementation into U-Boot.
>>>>>>>
>>>>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>>>>> ---
>>>>>>>   fs/ubifs/ubifs.h |    4 +---
>>>>>>>   include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>>>>>>   2 files changed, 41 insertions(+), 3 deletions(-)
>>>>>>>   create mode 100644 include/limits.h
>>>>>>
>>>>>> Apparently, in all the U-Boot codebase, only one file required standard
>>>>>> limit names, and gets them with three lines of code. Is it worth adding
>>>>>> 40
>>>>>> lines of code for this?
>>>>>
>>>>> Well 2/3 is the license header - and I thought it best to add all the
>>>>> limits in one go. I can add just those few if you like.
>>>>>
>>>>> This file is used later in the patch series.
>>>>
>>>> I don't see much use of these in the subsequent patches either -- and
>>>> those
>>>> few uses could be discussed, such as for instance the use of INT_MAX as
>>>> the
>>>> maximum buffer size for some *printf() functions -- actually, anything
>>>> very
>>>> big would fit just as well, would it not?
>>>
>>> Yes it would, it's doesn't really need to be INT_MAX. Then again,
>>> limits.h is a fairly standard file to have around, and INT_MAX is an
>>> efficient 'large' value to load on many architectures.
>>>
>>> In any case it seems wrong to me that ubifs is defining its own
>>> INT_MAX and U-Boot doesn't have one.
>>
>> My point is precisely that as standard as limits.h is, U-Boot has managed to
>> survive not having it around so far, which kind of shows limits.h is not
>> needed.
>
> By that logic one would never do any code clean ups. Do I understand
> you correctly?

You're extending my logic here: not all cleanups are done by adding a 
header file and replacing some lines by an include and some other lines. :)

Actually, I don't think introducing limits.h is any cleanup; it is an 
attempt at using standards whenever possible, which may be fine with 
some standards: I'd be happy of U-Boot used uint{8,16,32}_t instead of 
u{8,16,32}, for instance, because it uses that a lot. With limits.h, my 
gripe with it here is that, while possible, I don't see it bringing 
benefits here as 1) the ubi code already defines what it needs, 2) other 
uses in the patchset do not actually require /limits/, and 3) there are 
not many places in the whole U-Boot code that do.

If you can prove me wrong, i.e. if you can show that limits.h would add 
a lot of benefits to more than the other files in its own patchset, then 
I'll happily reconsider.

> But this is the least of my concerns :-) If you don't want it, don't
> take it. Shall I modify the series to define its own INT_MAX, or just
> chose a large number?

Well I don't think the limits.h introduction is useful here, and not 
introducing it will barely cost a source code line. As for the number to 
use in *printf(), either way is fine with me, as this number is 
arbitrary anyway.

> BTW I think you are looking at the old version of that patch series -
> we are now on v4. The limits.h patch is the same though. Later on in
> the series I add comments to vsprintf() functions and move them into
> their own header. If you apply the same logic there then that tidy is
> not needed also. Please let me know.

Thanks for reminding me. I did see the V4 series and it is the one I 
actually commented on in my previous mail. Apologies for not having made 
that explicit.

> Regards,
> Simon

Amicalement,
Simon Glass Oct. 22, 2011, 4:58 a.m. UTC | #8
Hi Albert,

On Fri, Oct 21, 2011 at 3:39 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 22/10/2011 00:02, Simon Glass a écrit :
>>
>> Hi Albert,
>>
>> On Fri, Oct 21, 2011 at 2:47 PM, Albert ARIBAUD
>> <albert.u.boot@aribaud.net>  wrote:
>>>
>>> Le 21/10/2011 23:12, Simon Glass a écrit :
>>>>
>>>> Hi Albert,
>>>>
>>>> On Fri, Oct 21, 2011 at 2:00 PM, Albert ARIBAUD
>>>> <albert.u.boot@aribaud.net>    wrote:
>>>>>
>>>>> Le 21/10/2011 22:19, Simon Glass a écrit :
>>>>>>
>>>>>> Hi Albert,
>>>>>>
>>>>>> On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
>>>>>> <albert.u.boot@aribaud.net>      wrote:
>>>>>>>
>>>>>>> Hi Simon,
>>>>>>>
>>>>>>> Le 10/10/2011 21:22, Simon Glass a écrit :
>>>>>>>>
>>>>>>>> This brings a basic limits.h implementation into U-Boot.
>>>>>>>>
>>>>>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>>>>>> ---
>>>>>>>>  fs/ubifs/ubifs.h |    4 +---
>>>>>>>>  include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>>>>>>>  2 files changed, 41 insertions(+), 3 deletions(-)
>>>>>>>>  create mode 100644 include/limits.h
>>>>>>>
>>>>>>> Apparently, in all the U-Boot codebase, only one file required
>>>>>>> standard
>>>>>>> limit names, and gets them with three lines of code. Is it worth
>>>>>>> adding
>>>>>>> 40
>>>>>>> lines of code for this?
>>>>>>
>>>>>> Well 2/3 is the license header - and I thought it best to add all the
>>>>>> limits in one go. I can add just those few if you like.
>>>>>>
>>>>>> This file is used later in the patch series.
>>>>>
>>>>> I don't see much use of these in the subsequent patches either -- and
>>>>> those
>>>>> few uses could be discussed, such as for instance the use of INT_MAX as
>>>>> the
>>>>> maximum buffer size for some *printf() functions -- actually, anything
>>>>> very
>>>>> big would fit just as well, would it not?
>>>>
>>>> Yes it would, it's doesn't really need to be INT_MAX. Then again,
>>>> limits.h is a fairly standard file to have around, and INT_MAX is an
>>>> efficient 'large' value to load on many architectures.
>>>>
>>>> In any case it seems wrong to me that ubifs is defining its own
>>>> INT_MAX and U-Boot doesn't have one.
>>>
>>> My point is precisely that as standard as limits.h is, U-Boot has managed
>>> to
>>> survive not having it around so far, which kind of shows limits.h is not
>>> needed.
>>
>> By that logic one would never do any code clean ups. Do I understand
>> you correctly?
>
> You're extending my logic here: not all cleanups are done by adding a header
> file and replacing some lines by an include and some other lines. :)
>
> Actually, I don't think introducing limits.h is any cleanup; it is an
> attempt at using standards whenever possible, which may be fine with some
> standards: I'd be happy of U-Boot used uint{8,16,32}_t instead of
> u{8,16,32}, for instance, because it uses that a lot. With limits.h, my
> gripe with it here is that, while possible, I don't see it bringing benefits
> here as 1) the ubi code already defines what it needs, 2) other uses in the
> patchset do not actually require /limits/, and 3) there are not many places
> in the whole U-Boot code that do.
>
> If you can prove me wrong, i.e. if you can show that limits.h would add a
> lot of benefits to more than the other files in its own patchset, then I'll
> happily reconsider.

I see few and small benefits. Of course if it is not there then people
will not use it, so it is self-fulfilling.

>
>> But this is the least of my concerns :-) If you don't want it, don't
>> take it. Shall I modify the series to define its own INT_MAX, or just
>> chose a large number?
>
> Well I don't think the limits.h introduction is useful here, and not
> introducing it will barely cost a source code line. As for the number to use
> in *printf(), either way is fine with me, as this number is arbitrary
> anyway.

OK

>
>> BTW I think you are looking at the old version of that patch series -
>> we are now on v4. The limits.h patch is the same though. Later on in
>> the series I add comments to vsprintf() functions and move them into
>> their own header. If you apply the same logic there then that tidy is
>> not needed also. Please let me know.
>
> Thanks for reminding me. I did see the V4 series and it is the one I
> actually commented on in my previous mail. Apologies for not having made
> that explicit.

OK that's fine - I will redo the series without limits.h.

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
>
Simon Glass Oct. 25, 2011, 11:43 p.m. UTC | #9
Hi Albert,

On Fri, Oct 21, 2011 at 9:58 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,
>
> On Fri, Oct 21, 2011 at 3:39 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Le 22/10/2011 00:02, Simon Glass a écrit :
>>>
>>> Hi Albert,
>>>
>>> On Fri, Oct 21, 2011 at 2:47 PM, Albert ARIBAUD
>>> <albert.u.boot@aribaud.net>  wrote:
>>>>
>>>> Le 21/10/2011 23:12, Simon Glass a écrit :
>>>>>
>>>>> Hi Albert,
>>>>>
>>>>> On Fri, Oct 21, 2011 at 2:00 PM, Albert ARIBAUD
>>>>> <albert.u.boot@aribaud.net>    wrote:
>>>>>>
>>>>>> Le 21/10/2011 22:19, Simon Glass a écrit :
>>>>>>>
>>>>>>> Hi Albert,
>>>>>>>
>>>>>>> On Fri, Oct 21, 2011 at 12:54 PM, Albert ARIBAUD
>>>>>>> <albert.u.boot@aribaud.net>      wrote:
>>>>>>>>
>>>>>>>> Hi Simon,
>>>>>>>>
>>>>>>>> Le 10/10/2011 21:22, Simon Glass a écrit :
>>>>>>>>>
>>>>>>>>> This brings a basic limits.h implementation into U-Boot.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>>>>>>>> ---
>>>>>>>>>  fs/ubifs/ubifs.h |    4 +---
>>>>>>>>>  include/limits.h |   40 ++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>  2 files changed, 41 insertions(+), 3 deletions(-)
>>>>>>>>>  create mode 100644 include/limits.h
>>>>>>>>
>>>>>>>> Apparently, in all the U-Boot codebase, only one file required
>>>>>>>> standard
>>>>>>>> limit names, and gets them with three lines of code. Is it worth
>>>>>>>> adding
>>>>>>>> 40
>>>>>>>> lines of code for this?
>>>>>>>
>>>>>>> Well 2/3 is the license header - and I thought it best to add all the
>>>>>>> limits in one go. I can add just those few if you like.
>>>>>>>
>>>>>>> This file is used later in the patch series.
>>>>>>
>>>>>> I don't see much use of these in the subsequent patches either -- and
>>>>>> those
>>>>>> few uses could be discussed, such as for instance the use of INT_MAX as
>>>>>> the
>>>>>> maximum buffer size for some *printf() functions -- actually, anything
>>>>>> very
>>>>>> big would fit just as well, would it not?
>>>>>
>>>>> Yes it would, it's doesn't really need to be INT_MAX. Then again,
>>>>> limits.h is a fairly standard file to have around, and INT_MAX is an
>>>>> efficient 'large' value to load on many architectures.
>>>>>
>>>>> In any case it seems wrong to me that ubifs is defining its own
>>>>> INT_MAX and U-Boot doesn't have one.
>>>>
>>>> My point is precisely that as standard as limits.h is, U-Boot has managed
>>>> to
>>>> survive not having it around so far, which kind of shows limits.h is not
>>>> needed.
>>>
>>> By that logic one would never do any code clean ups. Do I understand
>>> you correctly?
>>
>> You're extending my logic here: not all cleanups are done by adding a header
>> file and replacing some lines by an include and some other lines. :)
>>
>> Actually, I don't think introducing limits.h is any cleanup; it is an
>> attempt at using standards whenever possible, which may be fine with some
>> standards: I'd be happy of U-Boot used uint{8,16,32}_t instead of
>> u{8,16,32}, for instance, because it uses that a lot. With limits.h, my
>> gripe with it here is that, while possible, I don't see it bringing benefits
>> here as 1) the ubi code already defines what it needs, 2) other uses in the
>> patchset do not actually require /limits/, and 3) there are not many places
>> in the whole U-Boot code that do.
>>
>> If you can prove me wrong, i.e. if you can show that limits.h would add a
>> lot of benefits to more than the other files in its own patchset, then I'll
>> happily reconsider.
>
> I see few and small benefits. Of course if it is not there then people
> will not use it, so it is self-fulfilling.
>
>>
>>> But this is the least of my concerns :-) If you don't want it, don't
>>> take it. Shall I modify the series to define its own INT_MAX, or just
>>> chose a large number?
>>
>> Well I don't think the limits.h introduction is useful here, and not
>> introducing it will barely cost a source code line. As for the number to use
>> in *printf(), either way is fine with me, as this number is arbitrary
>> anyway.
>
> OK
>
>>
>>> BTW I think you are looking at the old version of that patch series -
>>> we are now on v4. The limits.h patch is the same though. Later on in
>>> the series I add comments to vsprintf() functions and move them into
>>> their own header. If you apply the same logic there then that tidy is
>>> not needed also. Please let me know.
>>
>> Thanks for reminding me. I did see the V4 series and it is the one I
>> actually commented on in my previous mail. Apologies for not having made
>> that explicit.
>
> OK that's fine - I will redo the series without limits.h.

Done - sent as v5.

Regards,
Simon

>
> Regards,
> Simon
>
>>
>>> Regards,
>>> Simon
>>
>> Amicalement,
>> --
>> Albert.
>>
>
Mike Frysinger Nov. 4, 2011, 2:33 a.m. UTC | #10
On Monday 10 October 2011 15:22:29 Simon Glass wrote:
> This brings a basic limits.h implementation into U-Boot.

sorry to jump in so late, but i think this was the correct way to go.  copying 
& pasting the same defines throughout the tree doesn't make much sense.  this 
would also make it easier to grab changes from Linux since they're the same.
-mike
Simon Glass Nov. 4, 2011, 5:14 a.m. UTC | #11
Hi Mike

On Thu, Nov 3, 2011 at 7:33 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday 10 October 2011 15:22:29 Simon Glass wrote:
>> This brings a basic limits.h implementation into U-Boot.
>
> sorry to jump in so late, but i think this was the correct way to go.  copying
> & pasting the same defines throughout the tree doesn't make much sense.  this
> would also make it easier to grab changes from Linux since they're the same.
> -mike
>

Well Albert was pretty keen on leaving this out. Next time we need
INT_MAX somewhere, or see someone defining it in a patch, we can think
about adding limits.h.

Regards,
Simon
Mike Frysinger Nov. 4, 2011, 11:09 p.m. UTC | #12
On Friday 04 November 2011 01:14:08 Simon Glass wrote:
> On Thu, Nov 3, 2011 at 7:33 PM, Mike Frysinger wrote:
> > On Monday 10 October 2011 15:22:29 Simon Glass wrote:
> >> This brings a basic limits.h implementation into U-Boot.
> > 
> > sorry to jump in so late, but i think this was the correct way to go.
> >  copying & pasting the same defines throughout the tree doesn't make
> > much sense.  this would also make it easier to grab changes from Linux
> > since they're the same.
> 
> Well Albert was pretty keen on leaving this out. Next time we need
> INT_MAX somewhere, or see someone defining it in a patch, we can think
> about adding limits.h.

his complaint was that there was one consumer.  we now have two.  and in both 
cases, it's because we imported code from Linux.

i would take the position that even if there is just one consumer (ubifs in 
this case), we should be importing this.  although it might make more sense to 
have this in linux/limits.h.  only downside there is that the upstream Linux 
code (oddly) places these in linux/kernel.h instead of linux/limits.h.  but i 
think i can live with that idiosyncrasy.
-mike
Simon Glass Nov. 5, 2011, 12:24 a.m. UTC | #13
Hi Mike,

On Fri, Nov 4, 2011 at 4:09 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Friday 04 November 2011 01:14:08 Simon Glass wrote:
>> On Thu, Nov 3, 2011 at 7:33 PM, Mike Frysinger wrote:
>> > On Monday 10 October 2011 15:22:29 Simon Glass wrote:
>> >> This brings a basic limits.h implementation into U-Boot.
>> >
>> > sorry to jump in so late, but i think this was the correct way to go.
>> >  copying & pasting the same defines throughout the tree doesn't make
>> > much sense.  this would also make it easier to grab changes from Linux
>> > since they're the same.
>>
>> Well Albert was pretty keen on leaving this out. Next time we need
>> INT_MAX somewhere, or see someone defining it in a patch, we can think
>> about adding limits.h.
>
> his complaint was that there was one consumer.  we now have two.  and in both
> cases, it's because we imported code from Linux.

My snprintf() patch series added the second so Albert was aware of
that. Also note it has not been merged yet.

>
> i would take the position that even if there is just one consumer (ubifs in
> this case), we should be importing this.  although it might make more sense to
> have this in linux/limits.h.  only downside there is that the upstream Linux
> code (oddly) places these in linux/kernel.h instead of linux/limits.h.  but i
> think i can live with that idiosyncrasy.
> -mike
>

Will let you and Albert discuss it over a beer :-)

Regards,
Simon
diff mbox

Patch

diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 0af471a..e7b6e43 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -311,9 +311,7 @@  struct file {
 #define MAX_LFS_FILESIZE 	0x7fffffffffffffffUL
 #endif
 
-#define INT_MAX		((int)(~0U>>1))
-#define INT_MIN		(-INT_MAX - 1)
-#define LLONG_MAX	((long long)(~0ULL>>1))
+#include <limits.h>
 
 /*
  * These are the fs-independent mount-flags: up to 32 flags are supported
diff --git a/include/limits.h b/include/limits.h
new file mode 100644
index 0000000..1021291
--- /dev/null
+++ b/include/limits.h
@@ -0,0 +1,40 @@ 
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __LIMITS_H
+#define __LIMITS_H
+
+/* Keep all our limits in one place */
+
+#define USHRT_MAX	((u16)(~0U))
+#define SHRT_MAX	((s16)(USHRT_MAX>>1))
+#define SHRT_MIN	((s16)(-SHRT_MAX - 1))
+#define INT_MAX		((int)(~0U>>1))
+#define INT_MIN		(-INT_MAX - 1)
+#define UINT_MAX	(~0U)
+#define LONG_MAX	((long)(~0UL>>1))
+#define LONG_MIN	(-LONG_MAX - 1)
+#define ULONG_MAX	(~0UL)
+#define LLONG_MAX	((long long)(~0ULL>>1))
+#define LLONG_MIN	(-LLONG_MAX - 1)
+#define ULLONG_MAX	(~0ULL)
+
+#endif