[U-Boot] [PATCH] Prevent malloc with size 0

[U-Boot] [PATCH] Prevent malloc with size 0

[Kostaras Nikolaos]

In case malloc is invoked with requested size 0, this patch will prevent
the execution of the allocation algorithm (because it corrupts the data 
structures)
and will return 0 to the caller.

Signed-off-by: Nikolaos Kostaras <nkost@intracomdefense.com>

---
 common/dlmalloc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/dlmalloc.c b/common/dlmalloc.c
index fce7a76..d9e3ea9 100644
--- a/common/dlmalloc.c
+++ b/common/dlmalloc.c
@@ -2182,7 +2182,7 @@ Void_t* mALLOc(bytes) size_t bytes;
     return 0;
   }

-  if ((long)bytes < 0) return 0;
+  if ((long)bytes <= 0) return 0;

   nb = request2size(bytes);  /* padded request size; */

--
1.6.4.4

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

> 
> In case malloc is invoked with requested size 0, this patch will prevent
> the execution of the allocation algorithm (because it corrupts the data 
> structures)
> and will return 0 to the caller.
> 
> Signed-off-by: Nikolaos Kostaras <nkost@intracomdefense.com>
> 
> ---
>  common/dlmalloc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/common/dlmalloc.c b/common/dlmalloc.c
> index fce7a76..d9e3ea9 100644
> --- a/common/dlmalloc.c
> +++ b/common/dlmalloc.c
> @@ -2182,7 +2182,7 @@ Void_t* mALLOc(bytes) size_t bytes;
>      return 0;
>    }
> 
> -  if ((long)bytes < 0) return 0;
> +  if ((long)bytes <= 0) return 0;

I think you should return some impossible ptr value =! NULL
Size 0 not really an error. 
In free you do:
if (impossible ptr)
   return;

If you can't find a good ptr value you could just do:
if (!bytes)
   bytes = 1;

[U-Boot] [PATCH] Prevent malloc with size 0

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF9263CF56.48468959-ONC12577C3.003E4D93-C12577C3.003ECAD7@transmode.se> you wrote:
>
> > -  if ((long)bytes < 0) return 0;
> > +  if ((long)bytes <= 0) return 0;
> 
> I think you should return some impossible ptr value =! NULL
> Size 0 not really an error. 

It is legal for malloc() to return NULL in case of size==0,
and for the sake of simplicity I recommend we do just that.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If the hours are long enough and the pay  is  short  enough,  someone
will say it's women's work.

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2010/10/21 13:32:54:
> 
> Dear Joakim Tjernlund,
> 
> In message <OF9263CF56.48468959-ONC12577C3.003E4D93-C12577C3.
> 003ECAD7 at transmode.se> you wrote:
> >
> > > -  if ((long)bytes < 0) return 0;
> > > +  if ((long)bytes <= 0) return 0;
> > 
> > I think you should return some impossible ptr value =! NULL
> > Size 0 not really an error. 
> 
> It is legal for malloc() to return NULL in case of size==0,
> and for the sake of simplicity I recommend we do just that.

Yes, but not very useful. Glibc does not return NULL

[U-Boot] [PATCH] Prevent malloc with size 0

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFD5ABFC5E.96E88C93-ONC12577C3.00406E0E-C12577C3.00408F11@transmode.se> you wrote:
>
> > It is legal for malloc() to return NULL in case of size==0,
> > and for the sake of simplicity I recommend we do just that.
> 
> Yes, but not very useful. Glibc does not return NULL

Maybe not in the current implementation, and not on the architecture
you checked. Current doc reads: "If size is 0, then malloc() returns
either NULL, or a unique pointer value that can later be successfully
passed to free()."

Of course we could return some valid pointer like glibc does, i. e.
implement something like

	if (size == 0)
		size = 8;

or so.  Do you think that would be better?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Horses just naturally have mohawk haircuts.

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2010/10/21 13:51:26:
> 
> Dear Joakim Tjernlund,
> 
> In message <OFD5ABFC5E.96E88C93-ONC12577C3.00406E0E-C12577C3.
> 00408F11 at transmode.se> you wrote:
> >
> > > It is legal for malloc() to return NULL in case of size==0,
> > > and for the sake of simplicity I recommend we do just that.
> > 
> > Yes, but not very useful. Glibc does not return NULL
> 
> Maybe not in the current implementation, and not on the architecture
> you checked. Current doc reads: "If size is 0, then malloc() returns
> either NULL, or a unique pointer value that can later be successfully
> passed to free()."

On linux and the tests I made, the unique pointer value is what I get.
Possibly it is configurable.

> 
> Of course we could return some valid pointer like glibc does, i. e.
> implement something like
> 
>    if (size == 0)
>       size = 8;
> 
> or so.  Do you think that would be better?

Better than NULL, but best would be a ptr that will SEGV if
you try to defer it. Not the easiest to impl., perhaps
~0 will do?

     Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF9AD66E3F.36E9C654-ONC12577C3.004134FD-C12577C3.0041A007@transmode.se> you wrote:
>
> > Of course we could return some valid pointer like glibc does, i. e.
> > implement something like
> > 
> >    if (size == 0)
> >       size = 8;
> > 
> > or so.  Do you think that would be better?
> 
> Better than NULL, but best would be a ptr that will SEGV if
> you try to defer it. Not the easiest to impl., perhaps
> ~0 will do?

The pointers you get from glibc can be read and written - they don't
segfault either (and usually we cannot do this in U-Boot, as most
systems have the MMU off).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
How many seconds are there in a year? If I tell you there are 3.155 x
10^7, you won't even try to remember it. On the other hand, who could
forget that, to within half a percent, pi seconds is  a  nanocentury.
                                               -- Tom Duff, Bell Labs

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2010/10/21 14:02:39:
> 
> Dear Joakim Tjernlund,
> 
> In message <OF9AD66E3F.36E9C654-ONC12577C3.004134FD-C12577C3.
> 0041A007 at transmode.se> you wrote:
> >
> > > Of course we could return some valid pointer like glibc does, i. e.
> > > implement something like
> > > 
> > >    if (size == 0)
> > >       size = 8;
> > > 
> > > or so.  Do you think that would be better?
> > 
> > Better than NULL, but best would be a ptr that will SEGV if
> > you try to defer it. Not the easiest to impl., perhaps
> > ~0 will do?
> 
> The pointers you get from glibc can be read and written - they don't
> segfault either (and usually we cannot do this in U-Boot, as most
> systems have the MMU off).

OK, then 
  if (size == 0)
       size = 8;
is just as good as glibc then.
A SEGV is better but you don't have to be perfect here

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Thursday, October 21, 2010 07:45:10 Joakim Tjernlund wrote:
> Wolfgang Denk wrote on 2010/10/21 13:32:54:
> > Joakim Tjernlund you wrote:
> > > > -  if ((long)bytes < 0) return 0;
> > > > +  if ((long)bytes <= 0) return 0;
> > > 
> > > I think you should return some impossible ptr value =! NULL
> > > Size 0 not really an error.
> > 
> > It is legal for malloc() to return NULL in case of size==0,
> > and for the sake of simplicity I recommend we do just that.
> 
> Yes, but not very useful. Glibc does not return NULL

it is useful for malloc(0) == NULL.  the glibc behavior is downright 
obnoxious.  we disable this for uClibc and dont see problems.  if anything, we 
catch accidental programming mistakes which then get fixed.

why exactly do you want malloc(0) to return valid memory ?  i would rather 
have u-boot return an error.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101021/751d818a/attachment.pgp 

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

On 22/10/10 06:51, Mike Frysinger wrote:
> On Thursday, October 21, 2010 07:45:10 Joakim Tjernlund wrote:
>> Wolfgang Denk wrote on 2010/10/21 13:32:54:
>>> Joakim Tjernlund you wrote:
>>>>> -  if ((long)bytes < 0) return 0;
>>>>> +  if ((long)bytes <= 0) return 0;
>>>>
>>>> I think you should return some impossible ptr value =! NULL
>>>> Size 0 not really an error.
>>>
>>> It is legal for malloc() to return NULL in case of size==0,
>>> and for the sake of simplicity I recommend we do just that.
>>
>> Yes, but not very useful. Glibc does not return NULL
> 
> it is useful for malloc(0) == NULL.  the glibc behavior is downright 
> obnoxious.  we disable this for uClibc and dont see problems.  if anything, we 
> catch accidental programming mistakes which then get fixed.
> 
> why exactly do you want malloc(0) to return valid memory ?  i would rather 

I agree

> have u-boot return an error.

Is NULL what you consider to be an error - in that case, I agree as well

Besides, is not free(NULL) valid (does nothing) as well?

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> On 22/10/10 06:51, Mike Frysinger wrote:
> > have u-boot return an error.
> 
> Is NULL what you consider to be an error

yes

> Besides, is not free(NULL) valid (does nothing) as well?

yes, free(NULL) should work fine per POSIX
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101021/f5390fbc/attachment.pgp 

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Mike Frysinger,

> On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> > On 22/10/10 06:51, Mike Frysinger wrote:
> > > have u-boot return an error.
> > 
> > Is NULL what you consider to be an error
> 
> yes
> 
> > Besides, is not free(NULL) valid (does nothing) as well?
> 
> yes, free(NULL) should work fine per POSIX
> -mike

Well then, this patch wasn't accepted yet and I consider it OK to apply. Any 
objections?

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

>
> Dear Mike Frysinger,
>
> > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> > > On 22/10/10 06:51, Mike Frysinger wrote:
> > > > have u-boot return an error.
> > >
> > > Is NULL what you consider to be an error
> >
> > yes
> >
> > > Besides, is not free(NULL) valid (does nothing) as well?
> >
> > yes, free(NULL) should work fine per POSIX
> > -mike
>
> Well then, this patch wasn't accepted yet and I consider it OK to apply. Any
> objections?

There was a long debate on the list regarding this where I argued that
malloc(0) should not be an error and malloc should return a ptr != NULL
I guess that is why it hasn't been applied.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> > Dear Mike Frysinger,
> > 
> > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> > > > On 22/10/10 06:51, Mike Frysinger wrote:
> > > > > have u-boot return an error.
> > > > 
> > > > Is NULL what you consider to be an error
> > > 
> > > yes
> > > 
> > > > Besides, is not free(NULL) valid (does nothing) as well?
> > > 
> > > yes, free(NULL) should work fine per POSIX
> > > -mike
> > 
> > Well then, this patch wasn't accepted yet and I consider it OK to apply.
> > Any objections?
> 
> There was a long debate on the list regarding this where I argued that
> malloc(0) should not be an error and malloc should return a ptr != NULL
> I guess that is why it hasn't been applied.
> 
>  Jocke

Ok, let's restart. Is there any objection why malloc(0) should not return NULL 
in uboot? Is it coliding with any spec?

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/01 16:01:56:
>
> Dear Joakim Tjernlund,
>
> > > Dear Mike Frysinger,
> > >
> > > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> > > > > On 22/10/10 06:51, Mike Frysinger wrote:
> > > > > > have u-boot return an error.
> > > > >
> > > > > Is NULL what you consider to be an error
> > > >
> > > > yes
> > > >
> > > > > Besides, is not free(NULL) valid (does nothing) as well?
> > > >
> > > > yes, free(NULL) should work fine per POSIX
> > > > -mike
> > >
> > > Well then, this patch wasn't accepted yet and I consider it OK to apply.
> > > Any objections?
> >
> > There was a long debate on the list regarding this where I argued that
> > malloc(0) should not be an error and malloc should return a ptr != NULL
> > I guess that is why it hasn't been applied.
> >
> >  Jocke
>
> Ok, let's restart. Is there any objection why malloc(0) should not return NULL
> in uboot?

Yes, read the thread to see why.

> Is it coliding with any spec?

No, both are valid.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/01 16:01:56:
> > Dear Joakim Tjernlund,
> > 
> > > > Dear Mike Frysinger,
> > > > 
> > > > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> > > > > > On 22/10/10 06:51, Mike Frysinger wrote:
> > > > > > > have u-boot return an error.
> > > > > > 
> > > > > > Is NULL what you consider to be an error
> > > > > 
> > > > > yes
> > > > > 
> > > > > > Besides, is not free(NULL) valid (does nothing) as well?
> > > > > 
> > > > > yes, free(NULL) should work fine per POSIX
> > > > > -mike
> > > > 
> > > > Well then, this patch wasn't accepted yet and I consider it OK to
> > > > apply. Any objections?
> > > 
> > > There was a long debate on the list regarding this where I argued that
> > > malloc(0) should not be an error and malloc should return a ptr != NULL
> > > I guess that is why it hasn't been applied.
> > > 
> > >  Jocke
> > 
> > Ok, let's restart. Is there any objection why malloc(0) should not return
> > NULL in uboot?
> 
> Yes, read the thread to see why.

Well I did, that's why I have no objections to applying this patch

> > Is it coliding with any spec?
> 
> No, both are valid.
> 
>  Jocke

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi All

Here we go again ;)

On Mon, Apr 2, 2012 at 12:21 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Joakim Tjernlund,
>
>> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/01 16:01:56:
>> > Dear Joakim Tjernlund,
>> >
>> > > > Dear Mike Frysinger,
>> > > >
>> > > > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
>> > > > > > On 22/10/10 06:51, Mike Frysinger wrote:
>> > > > > > > have u-boot return an error.
>> > > > > >
>> > > > > > Is NULL what you consider to be an error
>> > > > >
>> > > > > yes
>> > > > >
>> > > > > > Besides, is not free(NULL) valid (does nothing) as well?
>> > > > >
>> > > > > yes, free(NULL) should work fine per POSIX
>> > > > > -mike
>> > > >
>> > > > Well then, this patch wasn't accepted yet and I consider it OK to
>> > > > apply. Any objections?
>> > >
>> > > There was a long debate on the list regarding this where I argued that
>> > > malloc(0) should not be an error and malloc should return a ptr != NULL
>> > > I guess that is why it hasn't been applied.
>> > >
>> > > ?Jocke
>> >
>> > Ok, let's restart. Is there any objection why malloc(0) should not return
>> > NULL in uboot?
>>
>> Yes, read the thread to see why.
>
> Well I did, that's why I have no objections to applying this patch
>
>> > Is it coliding with any spec?
>>
>> No, both are valid.
>>

<quote author="Reinhard Meyer">
Out of principle I would say that malloc(0) should return a non-NULL
pointer of an area where exactly 0 bytes may be used. And, of course,
free() of that area shall not fail or crash the system.
</quote>

I'm wondering how exactly this would work - In theory, if you tried to
access this pointer you should get a segv. But I suppose if you malloc(1)
and try to access beyond the first byte there probably won't be a segv
either....

So to review the facts:

- The original complaint was that malloc(0) corrupts the malloc data
  structures, not that U-Boot's malloc(0) behaviour is non-standard
- Both the malloc(0) returns NULL and malloc(0) returns a uniquely
  free'able block of memory solutions are standard compliant
- malloc(0) returning NULL may break code which, for the sake of code
  simplicity, does not bother to check for zero-size before calling
  malloc()
- malloc(0) returning NULL may help to identify brain-dead use-cases

My vote:

        if ((long)bytes == 0) {
                DEBUG("Warning: malloc of zero block size\n");
                bytes = 1;
        } else if ((long)bytes < 0) {
                DEBUG("Error: malloc of negative block size\n");
                return 0;
        }

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Graeme Russ,

> Hi All
> 
> Here we go again ;)

Yay (polishing my flamethrower)!

> On Mon, Apr 2, 2012 at 12:21 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Joakim Tjernlund,
> > 
> >> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/01 16:01:56:
> >> > Dear Joakim Tjernlund,
> >> > 
> >> > > > Dear Mike Frysinger,
> >> > > > 
> >> > > > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> >> > > > > > On 22/10/10 06:51, Mike Frysinger wrote:
> >> > > > > > > have u-boot return an error.
> >> > > > > > 
> >> > > > > > Is NULL what you consider to be an error
> >> > > > > 
> >> > > > > yes
> >> > > > > 
> >> > > > > > Besides, is not free(NULL) valid (does nothing) as well?
> >> > > > > 
> >> > > > > yes, free(NULL) should work fine per POSIX
> >> > > > > -mike
> >> > > > 
> >> > > > Well then, this patch wasn't accepted yet and I consider it OK to
> >> > > > apply. Any objections?
> >> > > 
> >> > > There was a long debate on the list regarding this where I argued
> >> > > that malloc(0) should not be an error and malloc should return a
> >> > > ptr != NULL I guess that is why it hasn't been applied.
> >> > > 
> >> > >  Jocke
> >> > 
> >> > Ok, let's restart. Is there any objection why malloc(0) should not
> >> > return NULL in uboot?
> >> 
> >> Yes, read the thread to see why.
> > 
> > Well I did, that's why I have no objections to applying this patch
> > 
> >> > Is it coliding with any spec?
> >> 
> >> No, both are valid.
> 
> <quote author="Reinhard Meyer">
> Out of principle I would say that malloc(0) should return a non-NULL
> pointer of an area where exactly 0 bytes may be used. And, of course,
> free() of that area shall not fail or crash the system.
> </quote>
> 
> I'm wondering how exactly this would work - In theory, if you tried to
> access this pointer you should get a segv. But I suppose if you malloc(1)
> and try to access beyond the first byte there probably won't be a segv
> either....
> 
> So to review the facts:
> 
> - The original complaint was that malloc(0) corrupts the malloc data
>   structures, not that U-Boot's malloc(0) behaviour is non-standard
> - Both the malloc(0) returns NULL and malloc(0) returns a uniquely
>   free'able block of memory solutions are standard compliant
> - malloc(0) returning NULL may break code which, for the sake of code
>   simplicity, does not bother to check for zero-size before calling
>   malloc()

Well but you said malloc(0) corrupts the mallocator's data structures. Therefore 
malloc(0) used in code right now is broken anyway.

> - malloc(0) returning NULL may help to identify brain-dead use-cases

Agreed.

> 
> My vote:
> 
>         if ((long)bytes == 0) {
>                 DEBUG("Warning: malloc of zero block size\n");
>                 bytes = 1;

Well ... no, how can malloc(0) returning NULL break code that's already broken 
any more? It's silently roughing the mallocator structures up and it means the 
code is sitting on a ticking a-bomb anyway.

So we should add this like:

if (bytes == 0) {
	debug("You're sitting on a ticking A-Bomb doing this");
	return NULL;
} else if (bytes < 0) {
	return NULL;
}

>         } else if ((long)bytes < 0) {
>                 DEBUG("Error: malloc of negative block size\n");
>                 return 0;
>         }
> 
> Regards,
> 
> Graeme

Best regards,

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Marek,

On Mon, Apr 2, 2012 at 9:45 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Graeme Russ,
>
>> Hi All
>>
>> Here we go again ;)
>
> Yay (polishing my flamethrower)!
>
>> On Mon, Apr 2, 2012 at 12:21 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > Dear Joakim Tjernlund,
>> >
>> >> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/01 16:01:56:
>> >> > Dear Joakim Tjernlund,
>> >> >
>> >> > > > Dear Mike Frysinger,
>> >> > > >
>> >> > > > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
>> >> > > > > > On 22/10/10 06:51, Mike Frysinger wrote:
>> >> > > > > > > have u-boot return an error.
>> >> > > > > >
>> >> > > > > > Is NULL what you consider to be an error
>> >> > > > >
>> >> > > > > yes
>> >> > > > >
>> >> > > > > > Besides, is not free(NULL) valid (does nothing) as well?
>> >> > > > >
>> >> > > > > yes, free(NULL) should work fine per POSIX
>> >> > > > > -mike
>> >> > > >
>> >> > > > Well then, this patch wasn't accepted yet and I consider it OK to
>> >> > > > apply. Any objections?
>> >> > >
>> >> > > There was a long debate on the list regarding this where I argued
>> >> > > that malloc(0) should not be an error and malloc should return a
>> >> > > ptr != NULL I guess that is why it hasn't been applied.
>> >> > >
>> >> > > ?Jocke
>> >> >
>> >> > Ok, let's restart. Is there any objection why malloc(0) should not
>> >> > return NULL in uboot?
>> >>
>> >> Yes, read the thread to see why.
>> >
>> > Well I did, that's why I have no objections to applying this patch
>> >
>> >> > Is it coliding with any spec?
>> >>
>> >> No, both are valid.
>>
>> <quote author="Reinhard Meyer">
>> Out of principle I would say that malloc(0) should return a non-NULL
>> pointer of an area where exactly 0 bytes may be used. And, of course,
>> free() of that area shall not fail or crash the system.
>> </quote>
>>
>> I'm wondering how exactly this would work - In theory, if you tried to
>> access this pointer you should get a segv. But I suppose if you malloc(1)
>> and try to access beyond the first byte there probably won't be a segv
>> either....
>>
>> So to review the facts:
>>
>> - The original complaint was that malloc(0) corrupts the malloc data
>> ? structures, not that U-Boot's malloc(0) behaviour is non-standard
>> - Both the malloc(0) returns NULL and malloc(0) returns a uniquely
>> ? free'able block of memory solutions are standard compliant
>> - malloc(0) returning NULL may break code which, for the sake of code
>> ? simplicity, does not bother to check for zero-size before calling
>> ? malloc()
>
> Well but you said malloc(0) corrupts the mallocator's data structures. Therefore
> malloc(0) used in code right now is broken anyway.

Correct, but the breakage is in malloc() not the caller

>> - malloc(0) returning NULL may help to identify brain-dead use-cases
>
> Agreed.
>
>>
>> My vote:
>>
>> ? ? ? ? if ((long)bytes == 0) {
>> ? ? ? ? ? ? ? ? DEBUG("Warning: malloc of zero block size\n");
>> ? ? ? ? ? ? ? ? bytes = 1;
>
> Well ... no, how can malloc(0) returning NULL break code that's already broken
> any more? It's silently roughing the mallocator structures up and it means the
> code is sitting on a ticking a-bomb anyway.
>
> So we should add this like:
>
> if (bytes == 0) {
> ? ? ? ?debug("You're sitting on a ticking A-Bomb doing this");

Because you just set it off - Right now, that code is assuming malloc(0)
will return a valid pointer and thus not throw an E_NOMEM error - Now
all that code will fail with E_NOMEM

> ? ? ? ?return NULL;
> } else if (bytes < 0) {
> ? ? ? ?return NULL;
> }
>
>> ? ? ? ? } else if ((long)bytes < 0) {
>> ? ? ? ? ? ? ? ? DEBUG("Error: malloc of negative block size\n");
>> ? ? ? ? ? ? ? ? return 0;
>> ? ? ? ? }

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Graeme Russ,

> Hi Marek,
> 
> On Mon, Apr 2, 2012 at 9:45 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Graeme Russ,
> > 
> >> Hi All
> >> 
> >> Here we go again ;)
> > 
> > Yay (polishing my flamethrower)!
> > 
> >> On Mon, Apr 2, 2012 at 12:21 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> > Dear Joakim Tjernlund,
> >> > 
> >> >> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/01 16:01:56:
> >> >> > Dear Joakim Tjernlund,
> >> >> > 
> >> >> > > > Dear Mike Frysinger,
> >> >> > > > 
> >> >> > > > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote:
> >> >> > > > > > On 22/10/10 06:51, Mike Frysinger wrote:
> >> >> > > > > > > have u-boot return an error.
> >> >> > > > > > 
> >> >> > > > > > Is NULL what you consider to be an error
> >> >> > > > > 
> >> >> > > > > yes
> >> >> > > > > 
> >> >> > > > > > Besides, is not free(NULL) valid (does nothing) as well?
> >> >> > > > > 
> >> >> > > > > yes, free(NULL) should work fine per POSIX
> >> >> > > > > -mike
> >> >> > > > 
> >> >> > > > Well then, this patch wasn't accepted yet and I consider it OK
> >> >> > > > to apply. Any objections?
> >> >> > > 
> >> >> > > There was a long debate on the list regarding this where I argued
> >> >> > > that malloc(0) should not be an error and malloc should return a
> >> >> > > ptr != NULL I guess that is why it hasn't been applied.
> >> >> > > 
> >> >> > >  Jocke
> >> >> > 
> >> >> > Ok, let's restart. Is there any objection why malloc(0) should not
> >> >> > return NULL in uboot?
> >> >> 
> >> >> Yes, read the thread to see why.
> >> > 
> >> > Well I did, that's why I have no objections to applying this patch
> >> > 
> >> >> > Is it coliding with any spec?
> >> >> 
> >> >> No, both are valid.
> >> 
> >> <quote author="Reinhard Meyer">
> >> Out of principle I would say that malloc(0) should return a non-NULL
> >> pointer of an area where exactly 0 bytes may be used. And, of course,
> >> free() of that area shall not fail or crash the system.
> >> </quote>
> >> 
> >> I'm wondering how exactly this would work - In theory, if you tried to
> >> access this pointer you should get a segv. But I suppose if you
> >> malloc(1) and try to access beyond the first byte there probably won't
> >> be a segv either....
> >> 
> >> So to review the facts:
> >> 
> >> - The original complaint was that malloc(0) corrupts the malloc data
> >>   structures, not that U-Boot's malloc(0) behaviour is non-standard
> >> - Both the malloc(0) returns NULL and malloc(0) returns a uniquely
> >>   free'able block of memory solutions are standard compliant
> >> - malloc(0) returning NULL may break code which, for the sake of code
> >>   simplicity, does not bother to check for zero-size before calling
> >>   malloc()
> > 
> > Well but you said malloc(0) corrupts the mallocator's data structures.
> > Therefore malloc(0) used in code right now is broken anyway.
> 
> Correct, but the breakage is in malloc() not the caller

And what are the consequences of such a breakage?

> >> - malloc(0) returning NULL may help to identify brain-dead use-cases
> > 
> > Agreed.
> > 
> >> My vote:
> >> 
> >>         if ((long)bytes == 0) {
> >>                 DEBUG("Warning: malloc of zero block size\n");
> >>                 bytes = 1;
> > 
> > Well ... no, how can malloc(0) returning NULL break code that's already
> > broken any more? It's silently roughing the mallocator structures up and
> > it means the code is sitting on a ticking a-bomb anyway.
> > 
> > So we should add this like:
> > 
> > if (bytes == 0) {
> >        debug("You're sitting on a ticking A-Bomb doing this");
> 
> Because you just set it off - Right now, that code is assuming malloc(0)
> will return a valid pointer and thus not throw an E_NOMEM error - Now
> all that code will fail with E_NOMEM

Well ... that code worked with invalid memory (most probably not even R/W 
because it was some completely random hunk) and worked only by sheer 
coincidence. Let's break it, it was broken anyway.

Do you know about any such code? That's why I suggest adding such a debug() only 
in case there's malloc(0) called. Maybe even add a printf() instead.

> >        return NULL;
> > } else if (bytes < 0) {
> >        return NULL;
> > }
> > 
> >>         } else if ((long)bytes < 0) {
> >>                 DEBUG("Error: malloc of negative block size\n");
> >>                 return 0;
> >>         }
> 
> Regards,
> 
> Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Marek,

On Mon, Apr 2, 2012 at 10:13 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Graeme Russ,
>
>> Hi Marek,
>>
>> On Mon, Apr 2, 2012 at 9:45 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > Dear Graeme Russ,
>> >

>> Because you just set it off - Right now, that code is assuming malloc(0)
>> will return a valid pointer and thus not throw an E_NOMEM error - Now
>> all that code will fail with E_NOMEM
>
> Well ... that code worked with invalid memory (most probably not even R/W
> because it was some completely random hunk) and worked only by sheer
> coincidence. Let's break it, it was broken anyway.

a) The code calling malloc(0) is not broken, U-Boot's implementation of
   malloc(0) is.

b) The code calling malloc(0) is making a perfectly legitimate assumption
   based on how glibc handles malloc(0)

c) Just because glibc does something does not mean we have to

d) malloc(0) returning NULL and malloc(0) returning a valid pointer is not
   going to trouble me as I will never call malloc(0)

> Do you know about any such code? That's why I suggest adding such a debug() only
> in case there's malloc(0) called. Maybe even add a printf() instead.

Did you see the FDT example - Admitedly not in U-Boot but it's a really
good example IMHO - For the sake of code simplisity and clarity, some
processing loops are best implemented assuming malloc(0) will return
a valid pointer. Now if that pointer is de-referenced, then that is
the callers problem...


Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Graeme Russ,

> Hi Marek,
> 
> On Mon, Apr 2, 2012 at 10:13 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Graeme Russ,
> > 
> >> Hi Marek,
> >> 
> >> On Mon, Apr 2, 2012 at 9:45 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> > Dear Graeme Russ,
> >> 
> >> Because you just set it off - Right now, that code is assuming malloc(0)
> >> will return a valid pointer and thus not throw an E_NOMEM error - Now
> >> all that code will fail with E_NOMEM
> > 
> > Well ... that code worked with invalid memory (most probably not even R/W
> > because it was some completely random hunk) and worked only by sheer
> > coincidence. Let's break it, it was broken anyway.
> 
> a) The code calling malloc(0) is not broken, U-Boot's implementation of
>    malloc(0) is.

Well if it corrupts the internal structures of the mallocator, it's broken 
because it works by sheer coincidence. But I know what you wanna point out.

> b) The code calling malloc(0) is making a perfectly legitimate assumption
>    based on how glibc handles malloc(0)

Yes, agreed

> c) Just because glibc does something does not mean we have to

ACK

> d) malloc(0) returning NULL and malloc(0) returning a valid pointer is not
>    going to trouble me as I will never call malloc(0)

You sure? :)

Anyway, if we return something else than 0, how are we gonna trap such a null 
pointer?

> > Do you know about any such code? That's why I suggest adding such a
> > debug() only in case there's malloc(0) called. Maybe even add a printf()
> > instead.
> 
> Did you see the FDT example - Admitedly not in U-Boot but it's a really
> good example IMHO - For the sake of code simplisity and clarity, some
> processing loops are best implemented assuming malloc(0) will return
> a valid pointer. Now if that pointer is de-referenced, then that is
> the callers problem...

I did not see it, where?

> 
> Regards,
> 
> Graeme

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Marek,

On Mon, Apr 2, 2012 at 11:04 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Graeme Russ,
>
>> Hi Marek,
>>
>> On Mon, Apr 2, 2012 at 10:13 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > Dear Graeme Russ,
>> >
>> >> Hi Marek,
>> >>
>> >> On Mon, Apr 2, 2012 at 9:45 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> >> > Dear Graeme Russ,
>> >>
>> >> Because you just set it off - Right now, that code is assuming malloc(0)
>> >> will return a valid pointer and thus not throw an E_NOMEM error - Now
>> >> all that code will fail with E_NOMEM
>> >
>> > Well ... that code worked with invalid memory (most probably not even R/W
>> > because it was some completely random hunk) and worked only by sheer
>> > coincidence. Let's break it, it was broken anyway.
>>
>> a) The code calling malloc(0) is not broken, U-Boot's implementation of
>> ? ?malloc(0) is.
>
> Well if it corrupts the internal structures of the mallocator, it's broken
> because it works by sheer coincidence. But I know what you wanna point out.

If I call printf() with incorrect format specifiers and arguments (and the
compile does not pick it up) then my code is broken. If I call printf() and
the systems implementation does not support a standard format specifier
that I'm using then printf() is broken, not my code.

malloc(0) is a permissible call - It's unfortunate that the behaviour is
unspecified:

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf
ISO/IEC 9899:TC2 - May 6, 2005

7.20.3.3 The malloc function (p.314)
  The malloc function allocates space for an object whose size is specified
  by size and whose value is indeterminate.

 Returns
  The malloc function returns either a null pointer or a pointer to the
  allocated space.

J.1 Unspecified behavior (p. 490)
 - The amount of storage allocated by a successful call to the calloc,
   malloc, or realloc function when 0 bytes was requested (7.20.3)

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
ISO/IEC 9899:201x - April 12, 2011

7.22.3.4 The malloc function (p. 349)
 Synopsis
  #include <stdlib.h>
  void *malloc(size_t size);
 Description
  The malloc function allocates space for an object whose size is specified
  by size and whose value is indeterminate.
 Returns
  The malloc function returns either a null pointer or a pointer to the
  allocated space.

J.1 Unspecified behavior (p. 556)
 The amount of storage allocated by a successful call to the calloc,
 malloc, or realloc function when 0 bytes was requested (7.22.3).

I have also seen forum postings of the form:
 Section 7.20.3
  If the size of the space requested is zero, the behavior is
  implementation defined: either a null pointer is returned, or the
  behavior is as if the size were some nonzero value, except that the
  returned pointer shall not be used to access an object.

but have not seen an official document stating this

>> b) The code calling malloc(0) is making a perfectly legitimate assumption
>> ? ?based on how glibc handles malloc(0)
>
> Yes, agreed
>
>> c) Just because glibc does something does not mean we have to
>
> ACK
>
>> d) malloc(0) returning NULL and malloc(0) returning a valid pointer is not
>> ? ?going to trouble me as I will never call malloc(0)
>
> You sure? :)

No ;)

> Anyway, if we return something else than 0, how are we gonna trap such a null
> pointer?

You don't - You ask for a pointer to a block of memory of zero size and
malloc will return the smallest block it can. Remember, malloc(x) does not
have to return a block of exactly x bytes - it must return a block of at
least x bytes. It is up to you not to deference the pointer passed back
from malloc(0)

>> > Do you know about any such code? That's why I suggest adding such a
>> > debug() only in case there's malloc(0) called. Maybe even add a printf()
>> > instead.
>>
>> Did you see the FDT example - Admitedly not in U-Boot but it's a really
>> good example IMHO - For the sake of code simplisity and clarity, some
>> processing loops are best implemented assuming malloc(0) will return
>> a valid pointer. Now if that pointer is de-referenced, then that is
>> the callers problem...
>
> I did not see it, where?

patchwork (http://patchwork.ozlabs.org/patch/71914/)

Bottom line is, we could do either and we would be 100% compliant with the
C standard

The question is, what would be more onerous. Since the majority of U-Boot
developers will be more familiar with the glibc implementation, we may
one day end up with code that blindly assumes malloc(0) returns a valid
pointer and not NULL so to me, returning a valid pointer would be a logical
choice.

On the converse, returning NULL from malloc(0) means that any attempt to
(illegally) deference it will be immediately obvious...

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Graeme Russ,

> Hi Marek,
> 
> On Mon, Apr 2, 2012 at 11:04 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Graeme Russ,
> > 
> >> Hi Marek,
> >> 
> >> On Mon, Apr 2, 2012 at 10:13 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> > Dear Graeme Russ,
> >> > 
> >> >> Hi Marek,
> >> >> 
> >> >> On Mon, Apr 2, 2012 at 9:45 AM, Marek Vasut <marek.vasut@gmail.com> 
wrote:
> >> >> > Dear Graeme Russ,
> >> >> 
> >> >> Because you just set it off - Right now, that code is assuming
> >> >> malloc(0) will return a valid pointer and thus not throw an E_NOMEM
> >> >> error - Now all that code will fail with E_NOMEM
> >> > 
> >> > Well ... that code worked with invalid memory (most probably not even
> >> > R/W because it was some completely random hunk) and worked only by
> >> > sheer coincidence. Let's break it, it was broken anyway.
> >> 
> >> a) The code calling malloc(0) is not broken, U-Boot's implementation of
> >>    malloc(0) is.
> > 
> > Well if it corrupts the internal structures of the mallocator, it's
> > broken because it works by sheer coincidence. But I know what you wanna
> > point out.
> 
> If I call printf() with incorrect format specifiers and arguments (and the
> compile does not pick it up) then my code is broken. If I call printf() and
> the systems implementation does not support a standard format specifier
> that I'm using then printf() is broken, not my code.
> 
> malloc(0) is a permissible call - It's unfortunate that the behaviour is
> unspecified:
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf
> ISO/IEC 9899:TC2 - May 6, 2005
> 
> 7.20.3.3 The malloc function (p.314)
>   The malloc function allocates space for an object whose size is specified
>   by size and whose value is indeterminate.
> 
>  Returns
>   The malloc function returns either a null pointer or a pointer to the
>   allocated space.
> 
> J.1 Unspecified behavior (p. 490)
>  - The amount of storage allocated by a successful call to the calloc,
>    malloc, or realloc function when 0 bytes was requested (7.20.3)
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
> ISO/IEC 9899:201x - April 12, 2011
> 
> 7.22.3.4 The malloc function (p. 349)
>  Synopsis
>   #include <stdlib.h>
>   void *malloc(size_t size);
>  Description
>   The malloc function allocates space for an object whose size is specified
>   by size and whose value is indeterminate.
>  Returns
>   The malloc function returns either a null pointer or a pointer to the
>   allocated space.
> 
> J.1 Unspecified behavior (p. 556)
>  The amount of storage allocated by a successful call to the calloc,
>  malloc, or realloc function when 0 bytes was requested (7.22.3).
> 
> I have also seen forum postings of the form:
>  Section 7.20.3
>   If the size of the space requested is zero, the behavior is
>   implementation defined: either a null pointer is returned, or the
>   behavior is as if the size were some nonzero value, except that the
>   returned pointer shall not be used to access an object.
> 
> but have not seen an official document stating this
> 
> >> b) The code calling malloc(0) is making a perfectly legitimate
> >> assumption based on how glibc handles malloc(0)
> > 
> > Yes, agreed
> > 
> >> c) Just because glibc does something does not mean we have to
> > 
> > ACK
> > 
> >> d) malloc(0) returning NULL and malloc(0) returning a valid pointer is
> >> not going to trouble me as I will never call malloc(0)
> > 
> > You sure? :)
> 
> No ;)
> 
> > Anyway, if we return something else than 0, how are we gonna trap such a
> > null pointer?
> 
> You don't - You ask for a pointer to a block of memory of zero size and
> malloc will return the smallest block it can. Remember, malloc(x) does not
> have to return a block of exactly x bytes - it must return a block of at
> least x bytes. It is up to you not to deference the pointer passed back
> from malloc(0)
> 
> >> > Do you know about any such code? That's why I suggest adding such a
> >> > debug() only in case there's malloc(0) called. Maybe even add a
> >> > printf() instead.
> >> 
> >> Did you see the FDT example - Admitedly not in U-Boot but it's a really
> >> good example IMHO - For the sake of code simplisity and clarity, some
> >> processing loops are best implemented assuming malloc(0) will return
> >> a valid pointer. Now if that pointer is de-referenced, then that is
> >> the callers problem...
> > 
> > I did not see it, where?
> 
> patchwork (http://patchwork.ozlabs.org/patch/71914/)
> 
> Bottom line is, we could do either and we would be 100% compliant with the
> C standard
> 
> The question is, what would be more onerous. Since the majority of U-Boot
> developers will be more familiar with the glibc implementation, we may
> one day end up with code that blindly assumes malloc(0) returns a valid
> pointer and not NULL so to me, returning a valid pointer would be a logical
> choice.
> 
> On the converse, returning NULL from malloc(0) means that any attempt to
> (illegally) deference it will be immediately obvious...

So it's a question of being fool-proof vs. being compatible with glibc. This is 
a tough one, so what about voting ? ;-)

> Regards,
> 
> Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Marek,

On Mon, Apr 2, 2012 at 12:51 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Graeme Russ,
>
>> Hi Marek,

>> Bottom line is, we could do either and we would be 100% compliant with the
>> C standard
>>
>> The question is, what would be more onerous. Since the majority of U-Boot
>> developers will be more familiar with the glibc implementation, we may
>> one day end up with code that blindly assumes malloc(0) returns a valid
>> pointer and not NULL so to me, returning a valid pointer would be a logical
>> choice.
>>
>> On the converse, returning NULL from malloc(0) means that any attempt to
>> (illegally) deference it will be immediately obvious...
>
> So it's a question of being fool-proof vs. being compatible with glibc. This is
> a tough one, so what about voting ? ;-)
>

#define CONFIG_SYS_GLIBC_MALLOC_COMPAT

Kidding

Consider:

void some_function_a(void)
{
        uchar *blah;
        blah = malloc(1);

        if(blah)
                blah[1] = 'x';
        else
                printf("E_NOMEM\n");
}

void some_function_b(void)
{
        uchar *blah;
        blah = malloc(0);

        if(blah)
                blah[0] = 'x';
        else
                printf("E_NOMEM\n");
}

Both will corrupt data if malloc(0) returns a valid pointer. But:

void some_function_b(int i)
{
        uchar *blah;
        blah = malloc(i);

        if(blah)
                blah[i] = 'x';
        else
                printf("E_NOMEM\n");
}

Will return E_NOMEM if i == 0 and corrupt data is i > 0

It's inconsistent.

I originally thought returning NULL was the most appropriate option, but
seeing the FDT example and thinking how malloc(0) returning a valid pointer
has the potential to simplify (and hence make smaller and faster) code in
some circumstances.

My vote is for glibc compatibility

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 05:05:51:
>
> Hi Marek,
>
> On Mon, Apr 2, 2012 at 12:51 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Graeme Russ,
> >
> >> Hi Marek,
>
> >> Bottom line is, we could do either and we would be 100% compliant with the
> >> C standard
> >>
> >> The question is, what would be more onerous. Since the majority of U-Boot
> >> developers will be more familiar with the glibc implementation, we may
> >> one day end up with code that blindly assumes malloc(0) returns a valid
> >> pointer and not NULL so to me, returning a valid pointer would be a logical
> >> choice.
> >>
> >> On the converse, returning NULL from malloc(0) means that any attempt to
> >> (illegally) deference it will be immediately obvious...
> >
> > So it's a question of being fool-proof vs. being compatible with glibc. This is
> > a tough one, so what about voting ? ;-)
> >
>
> #define CONFIG_SYS_GLIBC_MALLOC_COMPAT
>
> Kidding
>
> Consider:
>
> void some_function_a(void)
> {
>         uchar *blah;
>         blah = malloc(1);
>
>         if(blah)
>                 blah[1] = 'x';
>         else
>                 printf("E_NOMEM\n");
> }
>
> void some_function_b(void)
> {
>         uchar *blah;
>         blah = malloc(0);
>
>         if(blah)
>                 blah[0] = 'x';
>         else
>                 printf("E_NOMEM\n");
> }
>
> Both will corrupt data if malloc(0) returns a valid pointer. But:
>
> void some_function_b(int i)
> {
>         uchar *blah;
>         blah = malloc(i);
>
>         if(blah)
>                 blah[i] = 'x';
>         else
>                 printf("E_NOMEM\n");
> }
>
> Will return E_NOMEM if i == 0 and corrupt data is i > 0
>
> It's inconsistent.
>
> I originally thought returning NULL was the most appropriate option, but
> seeing the FDT example and thinking how malloc(0) returning a valid pointer
> has the potential to simplify (and hence make smaller and faster) code in
> some circumstances.

:) exactly how I started too. I always figure malloc(0) should return NULL until
a few years ago when I tripped on a use case where it was inconvenient and it got me thinking.
It is like the early math systems were one didn't have 0 at all. You got away with it
for a long time but in the end one really need 0.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> b) The code calling malloc(0) is making a perfectly legitimate assumption
>    based on how glibc handles malloc(0)

not really.  POSIX says malloc(0) is implementation defined (so it may return a 
unique address, or it may return NULL).  no userspace code assuming malloc(0) 
will return non-NULL is correct.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120401/b120f6af/attachment.pgp>

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Mike,

On Mon, Apr 2, 2012 at 1:12 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
>> b) The code calling malloc(0) is making a perfectly legitimate assumption
>> ? ?based on how glibc handles malloc(0)
>
> not really. ?POSIX says malloc(0) is implementation defined (so it may return a
> unique address, or it may return NULL). ?no userspace code assuming malloc(0)
> will return non-NULL is correct.
> -mike

Argh! Valid point - So we can basically say that it does not matter what we
do (return NULL or return a valid pointer). Because the behaviour is
implementation specific, it is up to the caller to deal with it.

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Mike Frysinger,

> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > b) The code calling malloc(0) is making a perfectly legitimate assumption
> > 
> >    based on how glibc handles malloc(0)
> 
> not really.  POSIX says malloc(0) is implementation defined (so it may
> return a unique address, or it may return NULL).  no userspace code
> assuming malloc(0) will return non-NULL is correct.

Which is your implementation-defined ;-) But I have to agree with this one. So 
my vote is for returning NULL.

> -mike

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Marek,

On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Mike Frysinger,
>
>> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
>> > b) The code calling malloc(0) is making a perfectly legitimate assumption
>> >
>> > ? ?based on how glibc handles malloc(0)
>>
>> not really. ?POSIX says malloc(0) is implementation defined (so it may
>> return a unique address, or it may return NULL). ?no userspace code
>> assuming malloc(0) will return non-NULL is correct.
>
> Which is your implementation-defined ;-) But I have to agree with this one. So
> my vote is for returning NULL.

Also, no userspace code assuming malloc(0) will return NULL is correct

Point being, no matter which implementation is chosen, it is up to the
caller to not assume that the choice that was made was, in fact, the
choice that was made.

I.e. the behaviour of malloc(0) should be able to be changed on a whim
with no side-effects

So I think I should change my vote to returning NULL for one reason and
one reason only - It is faster during run-time

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Graeme Russ,

> Hi Marek,
> 
> On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Mike Frysinger,
> > 
> >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> >> > b) The code calling malloc(0) is making a perfectly legitimate
> >> > assumption
> >> > 
> >> >    based on how glibc handles malloc(0)
> >> 
> >> not really.  POSIX says malloc(0) is implementation defined (so it may
> >> return a unique address, or it may return NULL).  no userspace code
> >> assuming malloc(0) will return non-NULL is correct.
> > 
> > Which is your implementation-defined ;-) But I have to agree with this
> > one. So my vote is for returning NULL.
> 
> Also, no userspace code assuming malloc(0) will return NULL is correct
> 
> Point being, no matter which implementation is chosen, it is up to the
> caller to not assume that the choice that was made was, in fact, the
> choice that was made.
> 
> I.e. the behaviour of malloc(0) should be able to be changed on a whim
> with no side-effects
> 
> So I think I should change my vote to returning NULL for one reason and
> one reason only - It is faster during run-time

Well, this still might break some code which assumes otherwise ... like you 
said. And like I said, let's break it because it worked only be a sheer 
coincidence ;-)

> Regards,
> 
> Graeme

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Marek,

On Mon, Apr 2, 2012 at 2:23 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Graeme Russ,
>
>> Hi Marek,
>>
>> On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > Dear Mike Frysinger,
>> >
>> >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
>> >> > b) The code calling malloc(0) is making a perfectly legitimate
>> >> > assumption
>> >> >
>> >> > ? ?based on how glibc handles malloc(0)
>> >>
>> >> not really. ?POSIX says malloc(0) is implementation defined (so it may
>> >> return a unique address, or it may return NULL). ?no userspace code
>> >> assuming malloc(0) will return non-NULL is correct.
>> >
>> > Which is your implementation-defined ;-) But I have to agree with this
>> > one. So my vote is for returning NULL.
>>
>> Also, no userspace code assuming malloc(0) will return NULL is correct
>>
>> Point being, no matter which implementation is chosen, it is up to the
>> caller to not assume that the choice that was made was, in fact, the
>> choice that was made.
>>
>> I.e. the behaviour of malloc(0) should be able to be changed on a whim
>> with no side-effects
>>
>> So I think I should change my vote to returning NULL for one reason and
>> one reason only - It is faster during run-time
>
> Well, this still might break some code which assumes otherwise ... like you
> said. And like I said, let's break it because it worked only be a sheer
> coincidence ;-)

+1 your debug() and return NULL solution

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

>
> Hi Marek,
>
> On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Mike Frysinger,
> >
> >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> >> > b) The code calling malloc(0) is making a perfectly legitimate assumption
> >> >
> >> > ? ?based on how glibc handles malloc(0)
> >>
> >> not really. ?POSIX says malloc(0) is implementation defined (so it may
> >> return a unique address, or it may return NULL). ?no userspace code
> >> assuming malloc(0) will return non-NULL is correct.
> >
> > Which is your implementation-defined ;-) But I have to agree with this one. So
> > my vote is for returning NULL.
>
> Also, no userspace code assuming malloc(0) will return NULL is correct
>
> Point being, no matter which implementation is chosen, it is up to the
> caller to not assume that the choice that was made was, in fact, the
> choice that was made.
>
> I.e. the behaviour of malloc(0) should be able to be changed on a whim
> with no side-effects
>
> So I think I should change my vote to returning NULL for one reason and
> one reason only - It is faster during run-time

Then u-boot will be incompatible with both glibc and the linux kernel, it seems
to me that any modern impl. of malloc(0) will return a non NULL ptr.

It does need to be slower, just return ~0 instead, the kernel does something similar:
  if (!size)
     return ZERO_SIZE_PTR;

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Joakim,

On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se>
wrote:
>
> >
> > Hi Marek,
> >
> > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com>
wrote:
> > > Dear Mike Frysinger,
> > >
> > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > >> > b) The code calling malloc(0) is making a perfectly legitimate
assumption
> > >> >
> > >> >    based on how glibc handles malloc(0)
> > >>
> > >> not really.  POSIX says malloc(0) is implementation defined (so it
may
> > >> return a unique address, or it may return NULL).  no userspace code
> > >> assuming malloc(0) will return non-NULL is correct.
> > >
> > > Which is your implementation-defined ;-) But I have to agree with
this one. So
> > > my vote is for returning NULL.
> >
> > Also, no userspace code assuming malloc(0) will return NULL is correct
> >
> > Point being, no matter which implementation is chosen, it is up to the
> > caller to not assume that the choice that was made was, in fact, the
> > choice that was made.
> >
> > I.e. the behaviour of malloc(0) should be able to be changed on a whim
> > with no side-effects
> >
> > So I think I should change my vote to returning NULL for one reason and
> > one reason only - It is faster during run-time
>
> Then u-boot will be incompatible with both glibc and the linux kernel, it
seems

Forget aboug other implementations...

What matters is that the fact that the behaviour is undefined and it is up
to the caller to take that into account

> to me that any modern impl. of malloc(0) will return a non NULL ptr.
>
> It does need to be slower, just return ~0 instead, the kernel does
something similar:
>  if (!size)
>     return ZERO_SIZE_PTR;

That could work, but technically I don't think it complies as it is not a
pointer to allocated memory...

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Hi Grame

Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
>
> Hi Joakim,
> On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se> wrote:
> >
> > >
> > > Hi Marek,
> > >
> > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > > > Dear Mike Frysinger,
> > > >
> > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > >> > b) The code calling malloc(0) is making a perfectly legitimate assumption
> > > >> >
> > > >> > ? ?based on how glibc handles malloc(0)
> > > >>
> > > >> not really. ?POSIX says malloc(0) is implementation defined (so it may
> > > >> return a unique address, or it may return NULL). ?no userspace code
> > > >> assuming malloc(0) will return non-NULL is correct.
> > > >
> > > > Which is your implementation-defined ;-) But I have to agree with this one. So
> > > > my vote is for returning NULL.
> > >
> > > Also, no userspace code assuming malloc(0) will return NULL is correct
> > >
> > > Point being, no matter which implementation is chosen, it is up to the
> > > caller to not assume that the choice that was made was, in fact, the
> > > choice that was made.
> > >
> > > I.e. the behaviour of malloc(0) should be able to be changed on a whim
> > > with no side-effects
> > >
> > > So I think I should change my vote to returning NULL for one reason and
> > > one reason only - It is faster during run-time
> >
> > Then u-boot will be incompatible with both glibc and the linux kernel, it seems
> Forget aboug other implementations...
> What matters is that the fact that the behaviour is undefined and it is up to the caller to take that into account

Well, u-boot borrows code from both kernel and user space so it would make sense if
malloc(0) behaved the same. Especially for kernel code which tend to depend on the
kernels impl.(just look at Scotts example)

> > to me that any modern impl. of malloc(0) will return a non NULL ptr.
> >
> > It does need to be slower, just return ~0 instead, the kernel does something similar:
> > ?if (!size)
> > ? ? return ZERO_SIZE_PTR;
> That could work, but technically I don't think it complies as it is not a pointer to allocated memory...

It doesn't not have to be allocated memory, just a ptr != NULL which you can do free() on.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> Hi Grame
> 
> Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > Hi Joakim,
> > 
> > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se> 
wrote:
> > > > Hi Marek,
> > > > 
> > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> 
wrote:
> > > > > Dear Mike Frysinger,
> > > > > 
> > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > >> > b) The code calling malloc(0) is making a perfectly legitimate
> > > > >> > assumption
> > > > >> > 
> > > > >> >    based on how glibc handles malloc(0)
> > > > >> 
> > > > >> not really.  POSIX says malloc(0) is implementation defined (so it
> > > > >> may return a unique address, or it may return NULL).  no
> > > > >> userspace code assuming malloc(0) will return non-NULL is
> > > > >> correct.
> > > > > 
> > > > > Which is your implementation-defined ;-) But I have to agree with
> > > > > this one. So my vote is for returning NULL.
> > > > 
> > > > Also, no userspace code assuming malloc(0) will return NULL is
> > > > correct
> > > > 
> > > > Point being, no matter which implementation is chosen, it is up to
> > > > the caller to not assume that the choice that was made was, in fact,
> > > > the choice that was made.
> > > > 
> > > > I.e. the behaviour of malloc(0) should be able to be changed on a
> > > > whim with no side-effects
> > > > 
> > > > So I think I should change my vote to returning NULL for one reason
> > > > and one reason only - It is faster during run-time
> > > 
> > > Then u-boot will be incompatible with both glibc and the linux kernel,
> > > it seems
> > 
> > Forget aboug other implementations...
> > What matters is that the fact that the behaviour is undefined and it is
> > up to the caller to take that into account
> 
> Well, u-boot borrows code from both kernel and user space so it would make
> sense if malloc(0) behaved the same. Especially for kernel code which tend
> to depend on the kernels impl.(just look at Scotts example)
> 
> > > to me that any modern impl. of malloc(0) will return a non NULL ptr.
> > > 
> > > It does need to be slower, just return ~0 instead, the kernel does
> > > something similar: if (!size)
> > >     return ZERO_SIZE_PTR;
> > 
> > That could work, but technically I don't think it complies as it is not a
> > pointer to allocated memory...
> 
> It doesn't not have to be allocated memory, just a ptr != NULL which you
> can do free() on.

But kernel has something mapped there to trap these pointers I believe.

> 
>  Jocke

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
>
> Dear Joakim Tjernlund,
>
> > Hi Grame
> >
> > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > Hi Joakim,
> > >
> > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se>
> wrote:
> > > > > Hi Marek,
> > > > >
> > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com>
> wrote:
> > > > > > Dear Mike Frysinger,
> > > > > >
> > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > >> > b) The code calling malloc(0) is making a perfectly legitimate
> > > > > >> > assumption
> > > > > >> >
> > > > > >> >    based on how glibc handles malloc(0)
> > > > > >>
> > > > > >> not really.  POSIX says malloc(0) is implementation defined (so it
> > > > > >> may return a unique address, or it may return NULL).  no
> > > > > >> userspace code assuming malloc(0) will return non-NULL is
> > > > > >> correct.
> > > > > >
> > > > > > Which is your implementation-defined ;-) But I have to agree with
> > > > > > this one. So my vote is for returning NULL.
> > > > >
> > > > > Also, no userspace code assuming malloc(0) will return NULL is
> > > > > correct
> > > > >
> > > > > Point being, no matter which implementation is chosen, it is up to
> > > > > the caller to not assume that the choice that was made was, in fact,
> > > > > the choice that was made.
> > > > >
> > > > > I.e. the behaviour of malloc(0) should be able to be changed on a
> > > > > whim with no side-effects
> > > > >
> > > > > So I think I should change my vote to returning NULL for one reason
> > > > > and one reason only - It is faster during run-time
> > > >
> > > > Then u-boot will be incompatible with both glibc and the linux kernel,
> > > > it seems
> > >
> > > Forget aboug other implementations...
> > > What matters is that the fact that the behaviour is undefined and it is
> > > up to the caller to take that into account
> >
> > Well, u-boot borrows code from both kernel and user space so it would make
> > sense if malloc(0) behaved the same. Especially for kernel code which tend
> > to depend on the kernels impl.(just look at Scotts example)
> >
> > > > to me that any modern impl. of malloc(0) will return a non NULL ptr.
> > > >
> > > > It does need to be slower, just return ~0 instead, the kernel does
> > > > something similar: if (!size)
> > > >     return ZERO_SIZE_PTR;
> > >
> > > That could work, but technically I don't think it complies as it is not a
> > > pointer to allocated memory...
> >
> > It doesn't not have to be allocated memory, just a ptr != NULL which you
> > can do free() on.
>
> But kernel has something mapped there to trap these pointers I believe.

So? That only means that the kernel has extra protection if someone tries to
deference such a ptr. You are not required to do that(nice to have though)
You don have any protection for deferencing NULL either I think?

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > Dear Joakim Tjernlund,
> > 
> > > Hi Grame
> > > 
> > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > > Hi Joakim,
> > > > 
> > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > <joakim.tjernlund@transmode.se>
> > 
> > wrote:
> > > > > > Hi Marek,
> > > > > > 
> > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > <marek.vasut@gmail.com>
> > 
> > wrote:
> > > > > > > Dear Mike Frysinger,
> > > > > > > 
> > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > >> > b) The code calling malloc(0) is making a perfectly
> > > > > > >> > legitimate assumption
> > > > > > >> > 
> > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > >> 
> > > > > > >> not really.  POSIX says malloc(0) is implementation defined
> > > > > > >> (so it may return a unique address, or it may return NULL). 
> > > > > > >> no userspace code assuming malloc(0) will return non-NULL is
> > > > > > >> correct.
> > > > > > > 
> > > > > > > Which is your implementation-defined ;-) But I have to agree
> > > > > > > with this one. So my vote is for returning NULL.
> > > > > > 
> > > > > > Also, no userspace code assuming malloc(0) will return NULL is
> > > > > > correct
> > > > > > 
> > > > > > Point being, no matter which implementation is chosen, it is up
> > > > > > to the caller to not assume that the choice that was made was,
> > > > > > in fact, the choice that was made.
> > > > > > 
> > > > > > I.e. the behaviour of malloc(0) should be able to be changed on a
> > > > > > whim with no side-effects
> > > > > > 
> > > > > > So I think I should change my vote to returning NULL for one
> > > > > > reason and one reason only - It is faster during run-time
> > > > > 
> > > > > Then u-boot will be incompatible with both glibc and the linux
> > > > > kernel, it seems
> > > > 
> > > > Forget aboug other implementations...
> > > > What matters is that the fact that the behaviour is undefined and it
> > > > is up to the caller to take that into account
> > > 
> > > Well, u-boot borrows code from both kernel and user space so it would
> > > make sense if malloc(0) behaved the same. Especially for kernel code
> > > which tend to depend on the kernels impl.(just look at Scotts example)
> > > 
> > > > > to me that any modern impl. of malloc(0) will return a non NULL
> > > > > ptr.
> > > > > 
> > > > > It does need to be slower, just return ~0 instead, the kernel does
> > > > > something similar: if (!size)
> > > > > 
> > > > >     return ZERO_SIZE_PTR;
> > > > 
> > > > That could work, but technically I don't think it complies as it is
> > > > not a pointer to allocated memory...
> > > 
> > > It doesn't not have to be allocated memory, just a ptr != NULL which
> > > you can do free() on.
> > 
> > But kernel has something mapped there to trap these pointers I believe.
> 
> So? That only means that the kernel has extra protection if someone tries
> to deference such a ptr. You are not required to do that(nice to have
> though) You don have any protection for deferencing NULL either I think?

Can't GCC track it?

>  Jocke

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
>
> Dear Joakim Tjernlund,
>
> > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > > Dear Joakim Tjernlund,
> > >
> > > > Hi Grame
> > > >
> > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > > > Hi Joakim,
> > > > >
> > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > <joakim.tjernlund@transmode.se>
> > >
> > > wrote:
> > > > > > > Hi Marek,
> > > > > > >
> > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > <marek.vasut@gmail.com>
> > >
> > > wrote:
> > > > > > > > Dear Mike Frysinger,
> > > > > > > >
> > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > > >> > b) The code calling malloc(0) is making a perfectly
> > > > > > > >> > legitimate assumption
> > > > > > > >> >
> > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > >>
> > > > > > > >> not really.  POSIX says malloc(0) is implementation defined
> > > > > > > >> (so it may return a unique address, or it may return NULL).
> > > > > > > >> no userspace code assuming malloc(0) will return non-NULL is
> > > > > > > >> correct.
> > > > > > > >
> > > > > > > > Which is your implementation-defined ;-) But I have to agree
> > > > > > > > with this one. So my vote is for returning NULL.
> > > > > > >
> > > > > > > Also, no userspace code assuming malloc(0) will return NULL is
> > > > > > > correct
> > > > > > >
> > > > > > > Point being, no matter which implementation is chosen, it is up
> > > > > > > to the caller to not assume that the choice that was made was,
> > > > > > > in fact, the choice that was made.
> > > > > > >
> > > > > > > I.e. the behaviour of malloc(0) should be able to be changed on a
> > > > > > > whim with no side-effects
> > > > > > >
> > > > > > > So I think I should change my vote to returning NULL for one
> > > > > > > reason and one reason only - It is faster during run-time
> > > > > >
> > > > > > Then u-boot will be incompatible with both glibc and the linux
> > > > > > kernel, it seems
> > > > >
> > > > > Forget aboug other implementations...
> > > > > What matters is that the fact that the behaviour is undefined and it
> > > > > is up to the caller to take that into account
> > > >
> > > > Well, u-boot borrows code from both kernel and user space so it would
> > > > make sense if malloc(0) behaved the same. Especially for kernel code
> > > > which tend to depend on the kernels impl.(just look at Scotts example)
> > > >
> > > > > > to me that any modern impl. of malloc(0) will return a non NULL
> > > > > > ptr.
> > > > > >
> > > > > > It does need to be slower, just return ~0 instead, the kernel does
> > > > > > something similar: if (!size)
> > > > > >
> > > > > >     return ZERO_SIZE_PTR;
> > > > >
> > > > > That could work, but technically I don't think it complies as it is
> > > > > not a pointer to allocated memory...
> > > >
> > > > It doesn't not have to be allocated memory, just a ptr != NULL which
> > > > you can do free() on.
> > >
> > > But kernel has something mapped there to trap these pointers I believe.
> >
> > So? That only means that the kernel has extra protection if someone tries
> > to deference such a ptr. You are not required to do that(nice to have
> > though) You don have any protection for deferencing NULL either I think?
>
> Can't GCC track it?

Track what? NULL ptrs? I don't think so. Possibly when you have a static
NULL ptr but not in the general case.

I am getting tired of this discussion now. I am just trying to tell you that
no sane impl. of malloc() these days return NULL for malloc(0). Even though
standards allow it they don't consider malloc(0) an error, glibc will not
update errno in this case.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
> > Dear Joakim Tjernlund,
> > 
> > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > > > Dear Joakim Tjernlund,
> > > > 
> > > > > Hi Grame
> > > > > 
> > > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > > > > Hi Joakim,
> > > > > > 
> > > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > > <joakim.tjernlund@transmode.se>
> > > > 
> > > > wrote:
> > > > > > > > Hi Marek,
> > > > > > > > 
> > > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > > <marek.vasut@gmail.com>
> > > > 
> > > > wrote:
> > > > > > > > > Dear Mike Frysinger,
> > > > > > > > > 
> > > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > > > >> > b) The code calling malloc(0) is making a perfectly
> > > > > > > > >> > legitimate assumption
> > > > > > > > >> > 
> > > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > > >> 
> > > > > > > > >> not really.  POSIX says malloc(0) is implementation
> > > > > > > > >> defined (so it may return a unique address, or it may
> > > > > > > > >> return NULL). no userspace code assuming malloc(0) will
> > > > > > > > >> return non-NULL is correct.
> > > > > > > > > 
> > > > > > > > > Which is your implementation-defined ;-) But I have to
> > > > > > > > > agree with this one. So my vote is for returning NULL.
> > > > > > > > 
> > > > > > > > Also, no userspace code assuming malloc(0) will return NULL
> > > > > > > > is correct
> > > > > > > > 
> > > > > > > > Point being, no matter which implementation is chosen, it is
> > > > > > > > up to the caller to not assume that the choice that was made
> > > > > > > > was, in fact, the choice that was made.
> > > > > > > > 
> > > > > > > > I.e. the behaviour of malloc(0) should be able to be changed
> > > > > > > > on a whim with no side-effects
> > > > > > > > 
> > > > > > > > So I think I should change my vote to returning NULL for one
> > > > > > > > reason and one reason only - It is faster during run-time
> > > > > > > 
> > > > > > > Then u-boot will be incompatible with both glibc and the linux
> > > > > > > kernel, it seems
> > > > > > 
> > > > > > Forget aboug other implementations...
> > > > > > What matters is that the fact that the behaviour is undefined and
> > > > > > it is up to the caller to take that into account
> > > > > 
> > > > > Well, u-boot borrows code from both kernel and user space so it
> > > > > would make sense if malloc(0) behaved the same. Especially for
> > > > > kernel code which tend to depend on the kernels impl.(just look at
> > > > > Scotts example)
> > > > > 
> > > > > > > to me that any modern impl. of malloc(0) will return a non NULL
> > > > > > > ptr.
> > > > > > > 
> > > > > > > It does need to be slower, just return ~0 instead, the kernel
> > > > > > > does something similar: if (!size)
> > > > > > > 
> > > > > > >     return ZERO_SIZE_PTR;
> > > > > > 
> > > > > > That could work, but technically I don't think it complies as it
> > > > > > is not a pointer to allocated memory...
> > > > > 
> > > > > It doesn't not have to be allocated memory, just a ptr != NULL
> > > > > which you can do free() on.
> > > > 
> > > > But kernel has something mapped there to trap these pointers I
> > > > believe.
> > > 
> > > So? That only means that the kernel has extra protection if someone
> > > tries to deference such a ptr. You are not required to do that(nice to
> > > have though) You don have any protection for deferencing NULL either I
> > > think?
> > 
> > Can't GCC track it?
> 
> Track what? NULL ptrs? I don't think so. Possibly when you have a static
> NULL ptr but not in the general case.

Well of course.

> I am getting tired of this discussion now. I am just trying to tell you
> that no sane impl. of malloc() these days return NULL for malloc(0).

And I got your point. Though for u-boot, this would be the best solution 
actually. Anyone who uses memory allocated by malloc(0) is insane.

> Even
> though standards allow it they don't consider malloc(0) an error, glibc
> will not update errno in this case.

There's no errno in uboot I'm aware of ;-)

>  Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 17:23:03:
>
> Dear Joakim Tjernlund,
>
> > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
> > > Dear Joakim Tjernlund,
> > >
> > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > > > > Dear Joakim Tjernlund,
> > > > >
> > > > > > Hi Grame
> > > > > >
> > > > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > > > > > Hi Joakim,
> > > > > > >
> > > > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > > > <joakim.tjernlund@transmode.se>
> > > > >
> > > > > wrote:
> > > > > > > > > Hi Marek,
> > > > > > > > >
> > > > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > > > <marek.vasut@gmail.com>
> > > > >
> > > > > wrote:
> > > > > > > > > > Dear Mike Frysinger,
> > > > > > > > > >
> > > > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > > > > >> > b) The code calling malloc(0) is making a perfectly
> > > > > > > > > >> > legitimate assumption
> > > > > > > > > >> >
> > > > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > > > >>
> > > > > > > > > >> not really.  POSIX says malloc(0) is implementation
> > > > > > > > > >> defined (so it may return a unique address, or it may
> > > > > > > > > >> return NULL). no userspace code assuming malloc(0) will
> > > > > > > > > >> return non-NULL is correct.
> > > > > > > > > >
> > > > > > > > > > Which is your implementation-defined ;-) But I have to
> > > > > > > > > > agree with this one. So my vote is for returning NULL.
> > > > > > > > >
> > > > > > > > > Also, no userspace code assuming malloc(0) will return NULL
> > > > > > > > > is correct
> > > > > > > > >
> > > > > > > > > Point being, no matter which implementation is chosen, it is
> > > > > > > > > up to the caller to not assume that the choice that was made
> > > > > > > > > was, in fact, the choice that was made.
> > > > > > > > >
> > > > > > > > > I.e. the behaviour of malloc(0) should be able to be changed
> > > > > > > > > on a whim with no side-effects
> > > > > > > > >
> > > > > > > > > So I think I should change my vote to returning NULL for one
> > > > > > > > > reason and one reason only - It is faster during run-time
> > > > > > > >
> > > > > > > > Then u-boot will be incompatible with both glibc and the linux
> > > > > > > > kernel, it seems
> > > > > > >
> > > > > > > Forget aboug other implementations...
> > > > > > > What matters is that the fact that the behaviour is undefined and
> > > > > > > it is up to the caller to take that into account
> > > > > >
> > > > > > Well, u-boot borrows code from both kernel and user space so it
> > > > > > would make sense if malloc(0) behaved the same. Especially for
> > > > > > kernel code which tend to depend on the kernels impl.(just look at
> > > > > > Scotts example)
> > > > > >
> > > > > > > > to me that any modern impl. of malloc(0) will return a non NULL
> > > > > > > > ptr.
> > > > > > > >
> > > > > > > > It does need to be slower, just return ~0 instead, the kernel
> > > > > > > > does something similar: if (!size)
> > > > > > > >
> > > > > > > >     return ZERO_SIZE_PTR;
> > > > > > >
> > > > > > > That could work, but technically I don't think it complies as it
> > > > > > > is not a pointer to allocated memory...
> > > > > >
> > > > > > It doesn't not have to be allocated memory, just a ptr != NULL
> > > > > > which you can do free() on.
> > > > >
> > > > > But kernel has something mapped there to trap these pointers I
> > > > > believe.
> > > >
> > > > So? That only means that the kernel has extra protection if someone
> > > > tries to deference such a ptr. You are not required to do that(nice to
> > > > have though) You don have any protection for deferencing NULL either I
> > > > think?
> > >
> > > Can't GCC track it?
> >
> > Track what? NULL ptrs? I don't think so. Possibly when you have a static
> > NULL ptr but not in the general case.
>
> Well of course.

What did you mean then with "Can't GCC track it?" then? Just a bad joke?

>
> > I am getting tired of this discussion now. I am just trying to tell you
> > that no sane impl. of malloc() these days return NULL for malloc(0).
>
> And I got your point. Though for u-boot, this would be the best solution
> actually. Anyone who uses memory allocated by malloc(0) is insane.

No, you don't get the point. If you did you would not have have made the "insane" remark.

>
> > Even
> > though standards allow it they don't consider malloc(0) an error, glibc
> > will not update errno in this case.
>
> There's no errno in uboot I'm aware of ;-)

Just pointing out that malloc(0) is not an error even if malloc returns NULL in glibc/standards.
malloc(0) represents the empty set, just like 0 does in math and it is sometimes
useful.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 17:23:03:
> > Dear Joakim Tjernlund,
> > 
> > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
> > > > Dear Joakim Tjernlund,
> > > > 
> > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > > > > > Dear Joakim Tjernlund,
> > > > > > 
> > > > > > > Hi Grame
> > > > > > > 
> > > > > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > > > > > > Hi Joakim,
> > > > > > > > 
> > > > > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > > > > <joakim.tjernlund@transmode.se>
> > > > > > 
> > > > > > wrote:
> > > > > > > > > > Hi Marek,
> > > > > > > > > > 
> > > > > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > > > > <marek.vasut@gmail.com>
> > > > > > 
> > > > > > wrote:
> > > > > > > > > > > Dear Mike Frysinger,
> > > > > > > > > > > 
> > > > > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > > > > > >> > b) The code calling malloc(0) is making a perfectly
> > > > > > > > > > >> > legitimate assumption
> > > > > > > > > > >> > 
> > > > > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > > > > >> 
> > > > > > > > > > >> not really.  POSIX says malloc(0) is implementation
> > > > > > > > > > >> defined (so it may return a unique address, or it may
> > > > > > > > > > >> return NULL). no userspace code assuming malloc(0)
> > > > > > > > > > >> will return non-NULL is correct.
> > > > > > > > > > > 
> > > > > > > > > > > Which is your implementation-defined ;-) But I have to
> > > > > > > > > > > agree with this one. So my vote is for returning NULL.
> > > > > > > > > > 
> > > > > > > > > > Also, no userspace code assuming malloc(0) will return
> > > > > > > > > > NULL is correct
> > > > > > > > > > 
> > > > > > > > > > Point being, no matter which implementation is chosen, it
> > > > > > > > > > is up to the caller to not assume that the choice that
> > > > > > > > > > was made was, in fact, the choice that was made.
> > > > > > > > > > 
> > > > > > > > > > I.e. the behaviour of malloc(0) should be able to be
> > > > > > > > > > changed on a whim with no side-effects
> > > > > > > > > > 
> > > > > > > > > > So I think I should change my vote to returning NULL for
> > > > > > > > > > one reason and one reason only - It is faster during
> > > > > > > > > > run-time
> > > > > > > > > 
> > > > > > > > > Then u-boot will be incompatible with both glibc and the
> > > > > > > > > linux kernel, it seems
> > > > > > > > 
> > > > > > > > Forget aboug other implementations...
> > > > > > > > What matters is that the fact that the behaviour is undefined
> > > > > > > > and it is up to the caller to take that into account
> > > > > > > 
> > > > > > > Well, u-boot borrows code from both kernel and user space so it
> > > > > > > would make sense if malloc(0) behaved the same. Especially for
> > > > > > > kernel code which tend to depend on the kernels impl.(just look
> > > > > > > at Scotts example)
> > > > > > > 
> > > > > > > > > to me that any modern impl. of malloc(0) will return a non
> > > > > > > > > NULL ptr.
> > > > > > > > > 
> > > > > > > > > It does need to be slower, just return ~0 instead, the
> > > > > > > > > kernel does something similar: if (!size)
> > > > > > > > > 
> > > > > > > > >     return ZERO_SIZE_PTR;
> > > > > > > > 
> > > > > > > > That could work, but technically I don't think it complies as
> > > > > > > > it is not a pointer to allocated memory...
> > > > > > > 
> > > > > > > It doesn't not have to be allocated memory, just a ptr != NULL
> > > > > > > which you can do free() on.
> > > > > > 
> > > > > > But kernel has something mapped there to trap these pointers I
> > > > > > believe.
> > > > > 
> > > > > So? That only means that the kernel has extra protection if someone
> > > > > tries to deference such a ptr. You are not required to do that(nice
> > > > > to have though) You don have any protection for deferencing NULL
> > > > > either I think?
> > > > 
> > > > Can't GCC track it?
> > > 
> > > Track what? NULL ptrs? I don't think so. Possibly when you have a
> > > static NULL ptr but not in the general case.
> > 
> > Well of course.
> 
> What did you mean then with "Can't GCC track it?" then? Just a bad joke?

Never mind, didn't finish my train of thought.

> > > I am getting tired of this discussion now. I am just trying to tell you
> > > that no sane impl. of malloc() these days return NULL for malloc(0).
> > 
> > And I got your point. Though for u-boot, this would be the best solution
> > actually. Anyone who uses memory allocated by malloc(0) is insane.
> 
> No, you don't get the point. If you did you would not have have made the
> "insane" remark.

No, relying on malloc(0) returning something sane is messed up.

> > > Even
> > > though standards allow it they don't consider malloc(0) an error, glibc
> > > will not update errno in this case.
> > 
> > There's no errno in uboot I'm aware of ;-)
> 
> Just pointing out that malloc(0) is not an error even if malloc returns
> NULL in glibc/standards. malloc(0) represents the empty set, just like 0
> does in math and it is sometimes useful.
> 
>  Jocke

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 18:39:33:
> From: Marek Vasut <marek.vasut@gmail.com>
>
> Dear Joakim Tjernlund,
>
> > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 17:23:03:
> > > Dear Joakim Tjernlund,
> > >
> > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
> > > > > Dear Joakim Tjernlund,
> > > > >
> > > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > > > > > > Dear Joakim Tjernlund,
> > > > > > >
> > > > > > > > Hi Grame
> > > > > > > >
> > > > > > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > > > > > > > Hi Joakim,
> > > > > > > > >
> > > > > > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > > > > > <joakim.tjernlund@transmode.se>
> > > > > > >
> > > > > > > wrote:
> > > > > > > > > > > Hi Marek,
> > > > > > > > > > >
> > > > > > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > > > > > <marek.vasut@gmail.com>
> > > > > > >
> > > > > > > wrote:
> > > > > > > > > > > > Dear Mike Frysinger,
> > > > > > > > > > > >
> > > > > > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > > > > > > >> > b) The code calling malloc(0) is making a perfectly
> > > > > > > > > > > >> > legitimate assumption
> > > > > > > > > > > >> >
> > > > > > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > > > > > >>
> > > > > > > > > > > >> not really.  POSIX says malloc(0) is implementation
> > > > > > > > > > > >> defined (so it may return a unique address, or it may
> > > > > > > > > > > >> return NULL). no userspace code assuming malloc(0)
> > > > > > > > > > > >> will return non-NULL is correct.
> > > > > > > > > > > >
> > > > > > > > > > > > Which is your implementation-defined ;-) But I have to
> > > > > > > > > > > > agree with this one. So my vote is for returning NULL.
> > > > > > > > > > >
> > > > > > > > > > > Also, no userspace code assuming malloc(0) will return
> > > > > > > > > > > NULL is correct
> > > > > > > > > > >
> > > > > > > > > > > Point being, no matter which implementation is chosen, it
> > > > > > > > > > > is up to the caller to not assume that the choice that
> > > > > > > > > > > was made was, in fact, the choice that was made.
> > > > > > > > > > >
> > > > > > > > > > > I.e. the behaviour of malloc(0) should be able to be
> > > > > > > > > > > changed on a whim with no side-effects
> > > > > > > > > > >
> > > > > > > > > > > So I think I should change my vote to returning NULL for
> > > > > > > > > > > one reason and one reason only - It is faster during
> > > > > > > > > > > run-time
> > > > > > > > > >
> > > > > > > > > > Then u-boot will be incompatible with both glibc and the
> > > > > > > > > > linux kernel, it seems
> > > > > > > > >
> > > > > > > > > Forget aboug other implementations...
> > > > > > > > > What matters is that the fact that the behaviour is undefined
> > > > > > > > > and it is up to the caller to take that into account
> > > > > > > >
> > > > > > > > Well, u-boot borrows code from both kernel and user space so it
> > > > > > > > would make sense if malloc(0) behaved the same. Especially for
> > > > > > > > kernel code which tend to depend on the kernels impl.(just look
> > > > > > > > at Scotts example)
> > > > > > > >
> > > > > > > > > > to me that any modern impl. of malloc(0) will return a non
> > > > > > > > > > NULL ptr.
> > > > > > > > > >
> > > > > > > > > > It does need to be slower, just return ~0 instead, the
> > > > > > > > > > kernel does something similar: if (!size)
> > > > > > > > > >
> > > > > > > > > >     return ZERO_SIZE_PTR;
> > > > > > > > >
> > > > > > > > > That could work, but technically I don't think it complies as
> > > > > > > > > it is not a pointer to allocated memory...
> > > > > > > >
> > > > > > > > It doesn't not have to be allocated memory, just a ptr != NULL
> > > > > > > > which you can do free() on.
> > > > > > >
> > > > > > > But kernel has something mapped there to trap these pointers I
> > > > > > > believe.
> > > > > >
> > > > > > So? That only means that the kernel has extra protection if someone
> > > > > > tries to deference such a ptr. You are not required to do that(nice
> > > > > > to have though) You don have any protection for deferencing NULL
> > > > > > either I think?
> > > > >
> > > > > Can't GCC track it?
> > > >
> > > > Track what? NULL ptrs? I don't think so. Possibly when you have a
> > > > static NULL ptr but not in the general case.
> > >
> > > Well of course.
> >
> > What did you mean then with "Can't GCC track it?" then? Just a bad joke?
>
> Never mind, didn't finish my train of thought.

I almost figured that ...

>
> > > > I am getting tired of this discussion now. I am just trying to tell you
> > > > that no sane impl. of malloc() these days return NULL for malloc(0).
> > >
> > > And I got your point. Though for u-boot, this would be the best solution
> > > actually. Anyone who uses memory allocated by malloc(0) is insane.
> >
> > No, you don't get the point. If you did you would not have have made the
> > "insane" remark.
>
> No, relying on malloc(0) returning something sane is messed up.

Depends, if writing generic code for lots of OS:es you cannot rely malloc(0).
Writing kernel code you can. Not to mention those devs that don't
know better and just assumes that what works in glibc/kernel works every where.

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 18:39:33:
> > From: Marek Vasut <marek.vasut@gmail.com>
> > 
> > Dear Joakim Tjernlund,
> > 
> > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 17:23:03:
> > > > Dear Joakim Tjernlund,
> > > > 
> > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
> > > > > > Dear Joakim Tjernlund,
> > > > > > 
> > > > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > > > > > > > Dear Joakim Tjernlund,
> > > > > > > > 
> > > > > > > > > Hi Grame
> > > > > > > > > 
> > > > > > > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 
09:17:44:
> > > > > > > > > > Hi Joakim,
> > > > > > > > > > 
> > > > > > > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > > > > > > <joakim.tjernlund@transmode.se>
> > > > > > > > 
> > > > > > > > wrote:
> > > > > > > > > > > > Hi Marek,
> > > > > > > > > > > > 
> > > > > > > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > > > > > > <marek.vasut@gmail.com>
> > > > > > > > 
> > > > > > > > wrote:
> > > > > > > > > > > > > Dear Mike Frysinger,
> > > > > > > > > > > > > 
> > > > > > > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > > > > > > > >> > b) The code calling malloc(0) is making a
> > > > > > > > > > > > >> > perfectly legitimate assumption
> > > > > > > > > > > > >> > 
> > > > > > > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > > > > > > >> 
> > > > > > > > > > > > >> not really.  POSIX says malloc(0) is
> > > > > > > > > > > > >> implementation defined (so it may return a unique
> > > > > > > > > > > > >> address, or it may return NULL). no userspace
> > > > > > > > > > > > >> code assuming malloc(0) will return non-NULL is
> > > > > > > > > > > > >> correct.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Which is your implementation-defined ;-) But I have
> > > > > > > > > > > > > to agree with this one. So my vote is for
> > > > > > > > > > > > > returning NULL.
> > > > > > > > > > > > 
> > > > > > > > > > > > Also, no userspace code assuming malloc(0) will
> > > > > > > > > > > > return NULL is correct
> > > > > > > > > > > > 
> > > > > > > > > > > > Point being, no matter which implementation is
> > > > > > > > > > > > chosen, it is up to the caller to not assume that
> > > > > > > > > > > > the choice that was made was, in fact, the choice
> > > > > > > > > > > > that was made.
> > > > > > > > > > > > 
> > > > > > > > > > > > I.e. the behaviour of malloc(0) should be able to be
> > > > > > > > > > > > changed on a whim with no side-effects
> > > > > > > > > > > > 
> > > > > > > > > > > > So I think I should change my vote to returning NULL
> > > > > > > > > > > > for one reason and one reason only - It is faster
> > > > > > > > > > > > during run-time
> > > > > > > > > > > 
> > > > > > > > > > > Then u-boot will be incompatible with both glibc and
> > > > > > > > > > > the linux kernel, it seems
> > > > > > > > > > 
> > > > > > > > > > Forget aboug other implementations...
> > > > > > > > > > What matters is that the fact that the behaviour is
> > > > > > > > > > undefined and it is up to the caller to take that into
> > > > > > > > > > account
> > > > > > > > > 
> > > > > > > > > Well, u-boot borrows code from both kernel and user space
> > > > > > > > > so it would make sense if malloc(0) behaved the same.
> > > > > > > > > Especially for kernel code which tend to depend on the
> > > > > > > > > kernels impl.(just look at Scotts example)
> > > > > > > > > 
> > > > > > > > > > > to me that any modern impl. of malloc(0) will return a
> > > > > > > > > > > non NULL ptr.
> > > > > > > > > > > 
> > > > > > > > > > > It does need to be slower, just return ~0 instead, the
> > > > > > > > > > > kernel does something similar: if (!size)
> > > > > > > > > > > 
> > > > > > > > > > >     return ZERO_SIZE_PTR;
> > > > > > > > > > 
> > > > > > > > > > That could work, but technically I don't think it
> > > > > > > > > > complies as it is not a pointer to allocated memory...
> > > > > > > > > 
> > > > > > > > > It doesn't not have to be allocated memory, just a ptr !=
> > > > > > > > > NULL which you can do free() on.
> > > > > > > > 
> > > > > > > > But kernel has something mapped there to trap these pointers
> > > > > > > > I believe.
> > > > > > > 
> > > > > > > So? That only means that the kernel has extra protection if
> > > > > > > someone tries to deference such a ptr. You are not required to
> > > > > > > do that(nice to have though) You don have any protection for
> > > > > > > deferencing NULL either I think?
> > > > > > 
> > > > > > Can't GCC track it?
> > > > > 
> > > > > Track what? NULL ptrs? I don't think so. Possibly when you have a
> > > > > static NULL ptr but not in the general case.
> > > > 
> > > > Well of course.
> > > 
> > > What did you mean then with "Can't GCC track it?" then? Just a bad
> > > joke?
> > 
> > Never mind, didn't finish my train of thought.
> 
> I almost figured that ...
> 
> > > > > I am getting tired of this discussion now. I am just trying to tell
> > > > > you that no sane impl. of malloc() these days return NULL for
> > > > > malloc(0).
> > > > 
> > > > And I got your point. Though for u-boot, this would be the best
> > > > solution actually. Anyone who uses memory allocated by malloc(0) is
> > > > insane.
> > > 
> > > No, you don't get the point. If you did you would not have have made
> > > the "insane" remark.
> > 
> > No, relying on malloc(0) returning something sane is messed up.
> 
> Depends, if writing generic code for lots of OS:es you cannot rely
> malloc(0). Writing kernel code you can.

No you cannot. It's in the spec you cannot and you have to behave according to 
the spec, not according to kernel.

> Not to mention those devs that
> don't
> know better and just assumes that what works in glibc/kernel works every
> where.

Well, they will be taught it's not like that. Are we gonna support programmers 
who write crap code or what?

> From Scotts example we already know there is kernel code that relies on
> malloc(0) not returning NULL.

Sure, but that means the code is messed up.

> Your argument seems to boil down to "relying on malloc(0) returning
> something sane is messed up" so therefore u-boot malloc should take the
> easy route and just return NULL for malloc(0).

Basically, yes. It's correct according to the spec and we're not writing on 
operating system here, it's still a bootloader, so KISS.

> 
>       Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 20:00:03:
>
> Dear Joakim Tjernlund,
>
> > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 18:39:33:
> > > From: Marek Vasut <marek.vasut@gmail.com>
> > >
> > > Dear Joakim Tjernlund,
> > >
> > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 17:23:03:
> > > > > Dear Joakim Tjernlund,
> > > > >
> > > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
> > > > > > > Dear Joakim Tjernlund,
> > > > > > >
> > > > > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:05:13:
> > > > > > > > > Dear Joakim Tjernlund,
> > > > > > > > >
> > > > > > > > > > Hi Grame
> > > > > > > > > >
> > > > > > > > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02
> 09:17:44:
> > > > > > > > > > > Hi Joakim,
> > > > > > > > > > >
> > > > > > > > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > > > > > > > <joakim.tjernlund@transmode.se>
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > > > > > > Hi Marek,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > > > > > > > <marek.vasut@gmail.com>
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > > > > > > > Dear Mike Frysinger,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > > > > > > > > > > > >> > b) The code calling malloc(0) is making a
> > > > > > > > > > > > > >> > perfectly legitimate assumption
> > > > > > > > > > > > > >> >
> > > > > > > > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > > > > > > > >>
> > > > > > > > > > > > > >> not really.  POSIX says malloc(0) is
> > > > > > > > > > > > > >> implementation defined (so it may return a unique
> > > > > > > > > > > > > >> address, or it may return NULL). no userspace
> > > > > > > > > > > > > >> code assuming malloc(0) will return non-NULL is
> > > > > > > > > > > > > >> correct.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Which is your implementation-defined ;-) But I have
> > > > > > > > > > > > > > to agree with this one. So my vote is for
> > > > > > > > > > > > > > returning NULL.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Also, no userspace code assuming malloc(0) will
> > > > > > > > > > > > > return NULL is correct
> > > > > > > > > > > > >
> > > > > > > > > > > > > Point being, no matter which implementation is
> > > > > > > > > > > > > chosen, it is up to the caller to not assume that
> > > > > > > > > > > > > the choice that was made was, in fact, the choice
> > > > > > > > > > > > > that was made.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I.e. the behaviour of malloc(0) should be able to be
> > > > > > > > > > > > > changed on a whim with no side-effects
> > > > > > > > > > > > >
> > > > > > > > > > > > > So I think I should change my vote to returning NULL
> > > > > > > > > > > > > for one reason and one reason only - It is faster
> > > > > > > > > > > > > during run-time
> > > > > > > > > > > >
> > > > > > > > > > > > Then u-boot will be incompatible with both glibc and
> > > > > > > > > > > > the linux kernel, it seems
> > > > > > > > > > >
> > > > > > > > > > > Forget aboug other implementations...
> > > > > > > > > > > What matters is that the fact that the behaviour is
> > > > > > > > > > > undefined and it is up to the caller to take that into
> > > > > > > > > > > account
> > > > > > > > > >
> > > > > > > > > > Well, u-boot borrows code from both kernel and user space
> > > > > > > > > > so it would make sense if malloc(0) behaved the same.
> > > > > > > > > > Especially for kernel code which tend to depend on the
> > > > > > > > > > kernels impl.(just look at Scotts example)
> > > > > > > > > >
> > > > > > > > > > > > to me that any modern impl. of malloc(0) will return a
> > > > > > > > > > > > non NULL ptr.
> > > > > > > > > > > >
> > > > > > > > > > > > It does need to be slower, just return ~0 instead, the
> > > > > > > > > > > > kernel does something similar: if (!size)
> > > > > > > > > > > >
> > > > > > > > > > > >     return ZERO_SIZE_PTR;
> > > > > > > > > > >
> > > > > > > > > > > That could work, but technically I don't think it
> > > > > > > > > > > complies as it is not a pointer to allocated memory...
> > > > > > > > > >
> > > > > > > > > > It doesn't not have to be allocated memory, just a ptr !=
> > > > > > > > > > NULL which you can do free() on.
> > > > > > > > >
> > > > > > > > > But kernel has something mapped there to trap these pointers
> > > > > > > > > I believe.
> > > > > > > >
> > > > > > > > So? That only means that the kernel has extra protection if
> > > > > > > > someone tries to deference such a ptr. You are not required to
> > > > > > > > do that(nice to have though) You don have any protection for
> > > > > > > > deferencing NULL either I think?
> > > > > > >
> > > > > > > Can't GCC track it?
> > > > > >
> > > > > > Track what? NULL ptrs? I don't think so. Possibly when you have a
> > > > > > static NULL ptr but not in the general case.
> > > > >
> > > > > Well of course.
> > > >
> > > > What did you mean then with "Can't GCC track it?" then? Just a bad
> > > > joke?
> > >
> > > Never mind, didn't finish my train of thought.
> >
> > I almost figured that ...
> >
> > > > > > I am getting tired of this discussion now. I am just trying to tell
> > > > > > you that no sane impl. of malloc() these days return NULL for
> > > > > > malloc(0).
> > > > >
> > > > > And I got your point. Though for u-boot, this would be the best
> > > > > solution actually. Anyone who uses memory allocated by malloc(0) is
> > > > > insane.
> > > >
> > > > No, you don't get the point. If you did you would not have have made
> > > > the "insane" remark.
> > >
> > > No, relying on malloc(0) returning something sane is messed up.
> >
> > Depends, if writing generic code for lots of OS:es you cannot rely
> > malloc(0). Writing kernel code you can.
>
> No you cannot. It's in the spec you cannot and you have to behave according to
> the spec, not according to kernel.

How so? The kernel is its own system and has it own rules and it is wise
to follow them.

>
> > Not to mention those devs that
> > don't
> > know better and just assumes that what works in glibc/kernel works every
> > where.
>
> Well, they will be taught it's not like that. Are we gonna support programmers
> who write crap code or what?

You do either way, now you support those who assume malloc(0) returns NULL

>
> > From Scotts example we already know there is kernel code that relies on
> > malloc(0) not returning NULL.
>
> Sure, but that means the code is messed up.

ohh, I don't think the kernel people will agree: http://lwn.net/Articles/236920/
But feel free to bring it up.

>
> > Your argument seems to boil down to "relying on malloc(0) returning
> > something sane is messed up" so therefore u-boot malloc should take the
> > easy route and just return NULL for malloc(0).
>
> Basically, yes. It's correct according to the spec and we're not writing on
> operating system here, it's still a bootloader, so KISS.

Right, it is a boot loader and it reuses code from both kernel and the open source
community in general. So KISS here would be to follow suit.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Mon, Apr 2, 2012 at 14:40, Joakim Tjernlund wrote:
> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 20:00:03:
>> Dear Joakim Tjernlund,
>> > Depends, if writing generic code for lots of OS:es you cannot rely
>> > malloc(0). Writing kernel code you can.
>>
>> No you cannot. It's in the spec you cannot and you have to behave according to
>> the spec, not according to kernel.
>
> How so? The kernel is its own system and has it own rules and it is wise
> to follow them.

correct

>> > From Scotts example we already know there is kernel code that relies on
>> > malloc(0) not returning NULL.
>>
>> Sure, but that means the code is messed up.
>
> ohh, I don't think the kernel people will agree: http://lwn.net/Articles/236920/
> But feel free to bring it up.

i dislike the malloc(0) returning valid memory, but i'm fine with the
ZERO_SIZE_PTR idea.  i think we'd have to delegate this to arches
though to pick a pointer that'd work for them ... certainly the kernel
definition won't work for us:
  #define ZERO_SIZE_PTR ((void *)16)

address 0 and higher is valid memory on many platforms.  for Blackfin
systems, (-16) should work.
-mike

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

vapierfilter at gmail.com wrote on 2012/04/02 21:14:14:
>
> On Mon, Apr 2, 2012 at 14:40, Joakim Tjernlund wrote:
> > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 20:00:03:
> >> Dear Joakim Tjernlund,
> >> > Depends, if writing generic code for lots of OS:es you cannot rely
> >> > malloc(0). Writing kernel code you can.
> >>
> >> No you cannot. It's in the spec you cannot and you have to behave according to
> >> the spec, not according to kernel.
> >
> > How so? The kernel is its own system and has it own rules and it is wise
> > to follow them.
>
> correct
>
> >> > From Scotts example we already know there is kernel code that relies on
> >> > malloc(0) not returning NULL.
> >>
> >> Sure, but that means the code is messed up.
> >
> > ohh, I don't think the kernel people will agree: http://lwn.net/Articles/236920/
> > But feel free to bring it up.
>
> i dislike the malloc(0) returning valid memory, but i'm fine with the
> ZERO_SIZE_PTR idea.  i think we'd have to delegate this to arches
> though to pick a pointer that'd work for them ... certainly the kernel
> definition won't work for us:
>   #define ZERO_SIZE_PTR ((void *)16)
>
> address 0 and higher is valid memory on many platforms.  for Blackfin
> systems, (-16) should work.

Finding an invalid address would be ideal, but that might be hard even in arch code.
kmalloc(0) used to return a valid ptr up to 2.6.23 so this was never a big problem.
I guess one could try finding an invalid address in arch code anyway but let
board code override it if needed.

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

> Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 20:00:03:
> > Dear Joakim Tjernlund,
> > 
> > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 18:39:33:
> > > > From: Marek Vasut <marek.vasut@gmail.com>
> > > > 
> > > > Dear Joakim Tjernlund,
> > > > 
> > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 17:23:03:
> > > > > > Dear Joakim Tjernlund,
> > > > > > 
> > > > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 16:42:30:
> > > > > > > > Dear Joakim Tjernlund,
> > > > > > > > 
> > > > > > > > > Marek Vasut <marek.vasut@gmail.com> wrote on 2012/04/02 
16:05:13:
> > > > > > > > > > Dear Joakim Tjernlund,
> > > > > > > > > > 
> > > > > > > > > > > Hi Grame
> > > > > > > > > > > 
> > > > > > > > > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02
> > 
> > 09:17:44:
> > > > > > > > > > > > Hi Joakim,
> > > > > > > > > > > > 
> > > > > > > > > > > > On Apr 2, 2012 4:55 PM, "Joakim Tjernlund"
> > > > > > > > > > > > <joakim.tjernlund@transmode.se>
> > > > > > > > > > 
> > > > > > > > > > wrote:
> > > > > > > > > > > > > > Hi Marek,
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut
> > > > > > > > > > > > > > <marek.vasut@gmail.com>
> > > > > > > > > > 
> > > > > > > > > > wrote:
> > > > > > > > > > > > > > > Dear Mike Frysinger,
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > >> On Sunday 01 April 2012 20:25:44 Graeme Russ 
wrote:
> > > > > > > > > > > > > > >> > b) The code calling malloc(0) is making a
> > > > > > > > > > > > > > >> > perfectly legitimate assumption
> > > > > > > > > > > > > > >> > 
> > > > > > > > > > > > > > >> >    based on how glibc handles malloc(0)
> > > > > > > > > > > > > > >> 
> > > > > > > > > > > > > > >> not really.  POSIX says malloc(0) is
> > > > > > > > > > > > > > >> implementation defined (so it may return a
> > > > > > > > > > > > > > >> unique address, or it may return NULL). no
> > > > > > > > > > > > > > >> userspace code assuming malloc(0) will return
> > > > > > > > > > > > > > >> non-NULL is correct.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Which is your implementation-defined ;-) But I
> > > > > > > > > > > > > > > have to agree with this one. So my vote is for
> > > > > > > > > > > > > > > returning NULL.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Also, no userspace code assuming malloc(0) will
> > > > > > > > > > > > > > return NULL is correct
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Point being, no matter which implementation is
> > > > > > > > > > > > > > chosen, it is up to the caller to not assume that
> > > > > > > > > > > > > > the choice that was made was, in fact, the choice
> > > > > > > > > > > > > > that was made.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > I.e. the behaviour of malloc(0) should be able to
> > > > > > > > > > > > > > be changed on a whim with no side-effects
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > So I think I should change my vote to returning
> > > > > > > > > > > > > > NULL for one reason and one reason only - It is
> > > > > > > > > > > > > > faster during run-time
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Then u-boot will be incompatible with both glibc
> > > > > > > > > > > > > and the linux kernel, it seems
> > > > > > > > > > > > 
> > > > > > > > > > > > Forget aboug other implementations...
> > > > > > > > > > > > What matters is that the fact that the behaviour is
> > > > > > > > > > > > undefined and it is up to the caller to take that
> > > > > > > > > > > > into account
> > > > > > > > > > > 
> > > > > > > > > > > Well, u-boot borrows code from both kernel and user
> > > > > > > > > > > space so it would make sense if malloc(0) behaved the
> > > > > > > > > > > same. Especially for kernel code which tend to depend
> > > > > > > > > > > on the kernels impl.(just look at Scotts example)
> > > > > > > > > > > 
> > > > > > > > > > > > > to me that any modern impl. of malloc(0) will
> > > > > > > > > > > > > return a non NULL ptr.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > It does need to be slower, just return ~0 instead,
> > > > > > > > > > > > > the kernel does something similar: if (!size)
> > > > > > > > > > > > > 
> > > > > > > > > > > > >     return ZERO_SIZE_PTR;
> > > > > > > > > > > > 
> > > > > > > > > > > > That could work, but technically I don't think it
> > > > > > > > > > > > complies as it is not a pointer to allocated
> > > > > > > > > > > > memory...
> > > > > > > > > > > 
> > > > > > > > > > > It doesn't not have to be allocated memory, just a ptr
> > > > > > > > > > > != NULL which you can do free() on.
> > > > > > > > > > 
> > > > > > > > > > But kernel has something mapped there to trap these
> > > > > > > > > > pointers I believe.
> > > > > > > > > 
> > > > > > > > > So? That only means that the kernel has extra protection if
> > > > > > > > > someone tries to deference such a ptr. You are not required
> > > > > > > > > to do that(nice to have though) You don have any
> > > > > > > > > protection for deferencing NULL either I think?
> > > > > > > > 
> > > > > > > > Can't GCC track it?
> > > > > > > 
> > > > > > > Track what? NULL ptrs? I don't think so. Possibly when you have
> > > > > > > a static NULL ptr but not in the general case.
> > > > > > 
> > > > > > Well of course.
> > > > > 
> > > > > What did you mean then with "Can't GCC track it?" then? Just a bad
> > > > > joke?
> > > > 
> > > > Never mind, didn't finish my train of thought.
> > > 
> > > I almost figured that ...
> > > 
> > > > > > > I am getting tired of this discussion now. I am just trying to
> > > > > > > tell you that no sane impl. of malloc() these days return NULL
> > > > > > > for malloc(0).
> > > > > > 
> > > > > > And I got your point. Though for u-boot, this would be the best
> > > > > > solution actually. Anyone who uses memory allocated by malloc(0)
> > > > > > is insane.
> > > > > 
> > > > > No, you don't get the point. If you did you would not have have
> > > > > made the "insane" remark.
> > > > 
> > > > No, relying on malloc(0) returning something sane is messed up.
> > > 
> > > Depends, if writing generic code for lots of OS:es you cannot rely
> > > malloc(0). Writing kernel code you can.
> > 
> > No you cannot. It's in the spec you cannot and you have to behave
> > according to the spec, not according to kernel.
> 
> How so? The kernel is its own system and has it own rules and it is wise
> to follow them.

Why? We should follow the C spec first ;-)

> > > Not to mention those devs that
> > > don't
> > > know better and just assumes that what works in glibc/kernel works
> > > every where.
> > 
> > Well, they will be taught it's not like that. Are we gonna support
> > programmers who write crap code or what?
> 
> You do either way, now you support those who assume malloc(0) returns NULL

Is it the lesser of two evils or not? It certainly has benefits over allocating 
some small amount of data (speed).

> > > From Scotts example we already know there is kernel code that relies on
> > > malloc(0) not returning NULL.
> > 
> > Sure, but that means the code is messed up.
> 
> ohh, I don't think the kernel people will agree:
> http://lwn.net/Articles/236920/ But feel free to bring it up.

Ain't no fightin this with kernel folks.

> > > Your argument seems to boil down to "relying on malloc(0) returning
> > > something sane is messed up" so therefore u-boot malloc should take the
> > > easy route and just return NULL for malloc(0).
> > 
> > Basically, yes. It's correct according to the spec and we're not writing
> > on operating system here, it's still a bootloader, so KISS.
> 
> Right, it is a boot loader and it reuses code from both kernel and the open
> source community in general. So KISS here would be to follow suit.

You've got a valid point. On the other hand, if you return NULL, it'll be simple 
to find bugs. If you return a valid pointer, you'll get silent overwrites of 
memory (even mallocator structures), which is what bothers me :(

>  Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

On 04/02/2012 05:40 PM, Joakim Tjernlund wrote:
> Hi Grame
> 
> Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
>>
>> Hi Joakim,
>> On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se> wrote:
>>>
>>>>
>>>> Hi Marek,
>>>>
>>>> On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>> Dear Mike Frysinger,
>>>>>
>>>>>> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
>>>>>>> b) The code calling malloc(0) is making a perfectly legitimate assumption
>>>>>>>
>>>>>>>    based on how glibc handles malloc(0)
>>>>>>
>>>>>> not really.  POSIX says malloc(0) is implementation defined (so it may
>>>>>> return a unique address, or it may return NULL).  no userspace code
>>>>>> assuming malloc(0) will return non-NULL is correct.
>>>>>
>>>>> Which is your implementation-defined ;-) But I have to agree with this one. So
>>>>> my vote is for returning NULL.
>>>>
>>>> Also, no userspace code assuming malloc(0) will return NULL is correct
>>>>
>>>> Point being, no matter which implementation is chosen, it is up to the
>>>> caller to not assume that the choice that was made was, in fact, the
>>>> choice that was made.
>>>>
>>>> I.e. the behaviour of malloc(0) should be able to be changed on a whim
>>>> with no side-effects
>>>>
>>>> So I think I should change my vote to returning NULL for one reason and
>>>> one reason only - It is faster during run-time
>>>
>>> Then u-boot will be incompatible with both glibc and the linux kernel, it seems
>> Forget aboug other implementations...
>> What matters is that the fact that the behaviour is undefined and it is up to the caller to take that into account
> 
> Well, u-boot borrows code from both kernel and user space so it would make sense if
> malloc(0) behaved the same. Especially for kernel code which tend to depend on the
> kernels impl.(just look at Scotts example)
> 
>>> to me that any modern impl. of malloc(0) will return a non NULL ptr.
>>>
>>> It does need to be slower, just return ~0 instead, the kernel does something similar:
>>>  if (!size)
>>>     return ZERO_SIZE_PTR;
>> That could work, but technically I don't think it complies as it is not a pointer to allocated memory...
> 
> It doesn't not have to be allocated memory, just a ptr != NULL which you can do free() on.

As per the spec:

The malloc function returns either a null pointer or a pointer to the
allocated space.

The amount of storage allocated by a successful call to the calloc, malloc,
or realloc function when 0 bytes was requested (7.22.3).

The way I read that, if NULL is not returned, then what is returned is a
pointer to allocated space. If malloc(0) is called, the amount of space
allocated is not determined by the spec

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 22:28:46:
> From: Graeme Russ <graeme.russ@gmail.com>
>
> On 04/02/2012 05:40 PM, Joakim Tjernlund wrote:
> > Hi Grame
> >
> > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> >>
> >> Hi Joakim,
> >> On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se> wrote:
> >>>
> >>>>
> >>>> Hi Marek,
> >>>>
> >>>> On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>>> Dear Mike Frysinger,
> >>>>>
> >>>>>> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> >>>>>>> b) The code calling malloc(0) is making a perfectly legitimate assumption
> >>>>>>>
> >>>>>>>    based on how glibc handles malloc(0)
> >>>>>>
> >>>>>> not really.  POSIX says malloc(0) is implementation defined (so it may
> >>>>>> return a unique address, or it may return NULL).  no userspace code
> >>>>>> assuming malloc(0) will return non-NULL is correct.
> >>>>>
> >>>>> Which is your implementation-defined ;-) But I have to agree with this one. So
> >>>>> my vote is for returning NULL.
> >>>>
> >>>> Also, no userspace code assuming malloc(0) will return NULL is correct
> >>>>
> >>>> Point being, no matter which implementation is chosen, it is up to the
> >>>> caller to not assume that the choice that was made was, in fact, the
> >>>> choice that was made.
> >>>>
> >>>> I.e. the behaviour of malloc(0) should be able to be changed on a whim
> >>>> with no side-effects
> >>>>
> >>>> So I think I should change my vote to returning NULL for one reason and
> >>>> one reason only - It is faster during run-time
> >>>
> >>> Then u-boot will be incompatible with both glibc and the linux kernel, it seems
> >> Forget aboug other implementations...
> >> What matters is that the fact that the behaviour is undefined and it is up to the caller to take that into account
> >
> > Well, u-boot borrows code from both kernel and user space so it would make sense if
> > malloc(0) behaved the same. Especially for kernel code which tend to depend on the
> > kernels impl.(just look at Scotts example)
> >
> >>> to me that any modern impl. of malloc(0) will return a non NULL ptr.
> >>>
> >>> It does need to be slower, just return ~0 instead, the kernel does something similar:
> >>>  if (!size)
> >>>     return ZERO_SIZE_PTR;
> >> That could work, but technically I don't think it complies as it is not a pointer to allocated memory...
> >
> > It doesn't not have to be allocated memory, just a ptr != NULL which you can do free() on.
>
> As per the spec:
>
> The malloc function returns either a null pointer or a pointer to the
> allocated space.
>
> The amount of storage allocated by a successful call to the calloc, malloc,
> or realloc function when 0 bytes was requested (7.22.3).
>
> The way I read that, if NULL is not returned, then what is returned is a
> pointer to allocated space. If malloc(0) is called, the amount of space
> allocated is not determined by the spec

Please read http://lwn.net/Articles/236920/
They have a different view.

  Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

On Apr 3, 2012 6:57 AM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se>
wrote:
>
> Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 22:28:46:
> > From: Graeme Russ <graeme.russ@gmail.com>
> >
> > On 04/02/2012 05:40 PM, Joakim Tjernlund wrote:
> > > Hi Grame
> > >
> > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > >>
> > >> Hi Joakim,
> > >> On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <
joakim.tjernlund@transmode.se> wrote:
> > >>>
> > >>>>
> > >>>> Hi Marek,
> > >>>>
> > >>>> On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com>
wrote:
> > >>>>> Dear Mike Frysinger,
> > >>>>>
> > >>>>>> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > >>>>>>> b) The code calling malloc(0) is making a perfectly legitimate
assumption
> > >>>>>>>
> > >>>>>>>    based on how glibc handles malloc(0)
> > >>>>>>
> > >>>>>> not really.  POSIX says malloc(0) is implementation defined (so
it may
> > >>>>>> return a unique address, or it may return NULL).  no userspace
code
> > >>>>>> assuming malloc(0) will return non-NULL is correct.
> > >>>>>
> > >>>>> Which is your implementation-defined ;-) But I have to agree with
this one. So
> > >>>>> my vote is for returning NULL.
> > >>>>
> > >>>> Also, no userspace code assuming malloc(0) will return NULL is
correct
> > >>>>
> > >>>> Point being, no matter which implementation is chosen, it is up to
the
> > >>>> caller to not assume that the choice that was made was, in fact,
the
> > >>>> choice that was made.
> > >>>>
> > >>>> I.e. the behaviour of malloc(0) should be able to be changed on a
whim
> > >>>> with no side-effects
> > >>>>
> > >>>> So I think I should change my vote to returning NULL for one
reason and
> > >>>> one reason only - It is faster during run-time
> > >>>
> > >>> Then u-boot will be incompatible with both glibc and the linux
kernel, it seems
> > >> Forget aboug other implementations...
> > >> What matters is that the fact that the behaviour is undefined and it
is up to the caller to take that into account
> > >
> > > Well, u-boot borrows code from both kernel and user space so it would
make sense if
> > > malloc(0) behaved the same. Especially for kernel code which tend to
depend on the
> > > kernels impl.(just look at Scotts example)
> > >
> > >>> to me that any modern impl. of malloc(0) will return a non NULL ptr.
> > >>>
> > >>> It does need to be slower, just return ~0 instead, the kernel does
something similar:
> > >>>  if (!size)
> > >>>     return ZERO_SIZE_PTR;
> > >> That could work, but technically I don't think it complies as it is
not a pointer to allocated memory...
> > >
> > > It doesn't not have to be allocated memory, just a ptr != NULL which
you can do free() on.
> >
> > As per the spec:
> >
> > The malloc function returns either a null pointer or a pointer to the
> > allocated space.
> >
> > The amount of storage allocated by a successful call to the calloc,
malloc,
> > or realloc function when 0 bytes was requested (7.22.3).
> >
> > The way I read that, if NULL is not returned, then what is returned is a
> > pointer to allocated space. If malloc(0) is called, the amount of space
> > allocated is not determined by the spec
>
> Please read http://lwn.net/Articles/236920/
> They have a different view.

Yes, I read that. They also have a compelling argument.

Bottom line is, all three solutions are valid because, at the end of the
day, it's up to the caller to handle the unspecified behaviour.

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 22:59:57:
>
>
> On Apr 3, 2012 6:57 AM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se> wrote:
> >
> > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 22:28:46:
> > > From: Graeme Russ <graeme.russ@gmail.com>
> > >
> > > On 04/02/2012 05:40 PM, Joakim Tjernlund wrote:
> > > > Hi Grame
> > > >
> > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:
> > > >>
> > > >> Hi Joakim,
> > > >> On Apr 2, 2012 4:55 PM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se> wrote:
> > > >>>
> > > >>>>
> > > >>>> Hi Marek,
> > > >>>>
> > > >>>> On Mon, Apr 2, 2012 at 1:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > > >>>>> Dear Mike Frysinger,
> > > >>>>>
> > > >>>>>> On Sunday 01 April 2012 20:25:44 Graeme Russ wrote:
> > > >>>>>>> b) The code calling malloc(0) is making a perfectly legitimate assumption
> > > >>>>>>>
> > > >>>>>>> ? ?based on how glibc handles malloc(0)
> > > >>>>>>
> > > >>>>>> not really. ?POSIX says malloc(0) is implementation defined (so it may
> > > >>>>>> return a unique address, or it may return NULL). ?no userspace code
> > > >>>>>> assuming malloc(0) will return non-NULL is correct.
> > > >>>>>
> > > >>>>> Which is your implementation-defined ;-) But I have to agree with this one. So
> > > >>>>> my vote is for returning NULL.
> > > >>>>
> > > >>>> Also, no userspace code assuming malloc(0) will return NULL is correct
> > > >>>>
> > > >>>> Point being, no matter which implementation is chosen, it is up to the
> > > >>>> caller to not assume that the choice that was made was, in fact, the
> > > >>>> choice that was made.
> > > >>>>
> > > >>>> I.e. the behaviour of malloc(0) should be able to be changed on a whim
> > > >>>> with no side-effects
> > > >>>>
> > > >>>> So I think I should change my vote to returning NULL for one reason and
> > > >>>> one reason only - It is faster during run-time
> > > >>>
> > > >>> Then u-boot will be incompatible with both glibc and the linux kernel, it seems
> > > >> Forget aboug other implementations...
> > > >> What matters is that the fact that the behaviour is undefined and it is up to the caller to take that into account
> > > >
> > > > Well, u-boot borrows code from both kernel and user space so it would make sense if
> > > > malloc(0) behaved the same. Especially for kernel code which tend to depend on the
> > > > kernels impl.(just look at Scotts example)
> > > >
> > > >>> to me that any modern impl. of malloc(0) will return a non NULL ptr.
> > > >>>
> > > >>> It does need to be slower, just return ~0 instead, the kernel does something similar:
> > > >>> ?if (!size)
> > > >>> ? ? return ZERO_SIZE_PTR;
> > > >> That could work, but technically I don't think it complies as it is not a pointer to allocated memory...
> > > >
> > > > It doesn't not have to be allocated memory, just a ptr != NULL which you can do free() on.
> > >
> > > As per the spec:
> > >
> > > The malloc function returns either a null pointer or a pointer to the
> > > allocated space.
> > >
> > > The amount of storage allocated by a successful call to the calloc, malloc,
> > > or realloc function when 0 bytes was requested (7.22.3).
> > >
> > > The way I read that, if NULL is not returned, then what is returned is a
> > > pointer to allocated space. If malloc(0) is called, the amount of space
> > > allocated is not determined by the spec
> >
> > Please read http://lwn.net/Articles/236920/
> > They have a different view.
> Yes, I read that. They also have a compelling argument.
> Bottom line is, all three solutions are valid because, at the end of the day, it's up to the caller to handle the unspecified behaviour.

If you write code in general yes, for kernel no. We also know that many devs. doesn't know there is a
difference.
This is not really the question here though. It is: what method should u-boot malloc impl.?
I say that selecting NULL is the worst(in this long thread I have motivated why too)

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi?Jocke

On Tue, Apr 3, 2012 at 7:14 AM, Joakim Tjernlund
<joakim.tjernlund@transmode.se> wrote:
> Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 22:59:57:
>>
>>
>> On Apr 3, 2012 6:57 AM, "Joakim Tjernlund" <joakim.tjernlund@transmode.se> wrote:
>> >
>> > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 22:28:46:
>> > > From: Graeme Russ <graeme.russ@gmail.com>
>> > >
>> > > On 04/02/2012 05:40 PM, Joakim Tjernlund wrote:
>> > > > Hi Grame
>> > > >
>> > > > Graeme Russ <graeme.russ@gmail.com> wrote on 2012/04/02 09:17:44:

[snip]

>> > > >> That could work, but technically I don't think it complies as it is not a pointer to allocated memory...
>> > > >
>> > > > It doesn't not have to be allocated memory, just a ptr != NULL which you can do free() on.
>> > >
>> > > As per the spec:
>> > >
>> > > The malloc function returns either a null pointer or a pointer to the
>> > > allocated space.
>> > >
>> > > The amount of storage allocated by a successful call to the calloc, malloc,
>> > > or realloc function when 0 bytes was requested (7.22.3).
>> > >
>> > > The way I read that, if NULL is not returned, then what is returned is a
>> > > pointer to allocated space. If malloc(0) is called, the amount of space
>> > > allocated is not determined by the spec
>> >
>> > Please read http://lwn.net/Articles/236920/
>> > They have a different view.
>> Yes, I read that. They also have a compelling argument.
>> Bottom line is, all three solutions are valid because, at the end of the day, it's up to the caller to handle the unspecified behaviour.
>
> If you write code in general yes, for kernel no. We also know that many devs. doesn't know there is a
> difference.
> This is not really the question here though. It is: what method should u-boot malloc impl.?
> I say that selecting NULL is the worst(in this long thread I have motivated why too)

This thread both fascinates and annoys me...

I've changed my position on this issue several times, which to me indicates
that it does not really bother me that much which way we go...

As the kernel guys point out, returning non-NULL has a distinct code
simplicity advantage (especially when compile-time options can reduce some
data structures or other allocations to zero size)

And I really need to check, but I have a sneaking suspicion that as the
code currently stands in U-Boot/x86 dereferencing a NULL pointer won't
cause an exception. In x86, U-Boot configures all protected mode segments
to be 4GB starting at physical address 0x00000000 with no virtual address
translation. Accessing physical address 0x00000000 is just as valid as
accessing 0x00000001 (or any other address).

Now if I set segments to start at 0x00000002 then I can trap a segmentation
fault for accesses to 0x00000000 (NULL) and 0x00000001 (malloc(0) pointer)

That will mean that U-Boot cannot ever access those two bytes of memory,
but I doubt that I would ever want to. And I will need to set the segments
to base address 0x00000000 before jumping into Linux...

Let me have a play tonight

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

On 04/03/2012 09:35 AM, Graeme Russ wrote:
> Hi Jocke

> And I really need to check, but I have a sneaking suspicion that as the
> code currently stands in U-Boot/x86 dereferencing a NULL pointer won't
> cause an exception. In x86, U-Boot configures all protected mode segments
> to be 4GB starting at physical address 0x00000000 with no virtual address
> translation. Accessing physical address 0x00000000 is just as valid as
> accessing 0x00000001 (or any other address).
> 
> Now if I set segments to start at 0x00000002 then I can trap a segmentation
> fault for accesses to 0x00000000 (NULL) and 0x00000001 (malloc(0) pointer)
> 
> That will mean that U-Boot cannot ever access those two bytes of memory,
> but I doubt that I would ever want to. And I will need to set the segments
> to base address 0x00000000 before jumping into Linux...

OK, this is not as easy as it sounds. Detecting NULL pointer dereferences
will involve enabling paging[1] which is something I really do not want to
do in U-Boot. Flat Protected Mode with a 4GB linear map is perfectly fit
for purpose, and that is how the Linux kernel expects things to be
configured so it will be a major PITA to change.

In short, returning non-NULL from malloc(0) and expecting a CPU exception
when it is de-referenced is not going to fly.

If we choose this path, at least put a debug() statement in to warn when
malloc(0) is called.

Regards,

Graeme

[1] Apparently the way do do it is to reserve the entire first 4kB page and
mark it as 'not-present' so any access causes a page-fault.

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Graeme Russ,

> On 04/03/2012 09:35 AM, Graeme Russ wrote:
> > Hi Jocke
> > 
> > And I really need to check, but I have a sneaking suspicion that as the
> > code currently stands in U-Boot/x86 dereferencing a NULL pointer won't
> > cause an exception. In x86, U-Boot configures all protected mode segments
> > to be 4GB starting at physical address 0x00000000 with no virtual address
> > translation. Accessing physical address 0x00000000 is just as valid as
> > accessing 0x00000001 (or any other address).
> > 
> > Now if I set segments to start at 0x00000002 then I can trap a
> > segmentation fault for accesses to 0x00000000 (NULL) and 0x00000001
> > (malloc(0) pointer)
> > 
> > That will mean that U-Boot cannot ever access those two bytes of memory,
> > but I doubt that I would ever want to. And I will need to set the
> > segments to base address 0x00000000 before jumping into Linux...
> 
> OK, this is not as easy as it sounds. Detecting NULL pointer dereferences
> will involve enabling paging[1] which is something I really do not want to
> do in U-Boot. Flat Protected Mode with a 4GB linear map is perfectly fit
> for purpose, and that is how the Linux kernel expects things to be
> configured so it will be a major PITA to change.
> 
> In short, returning non-NULL from malloc(0) and expecting a CPU exception
> when it is de-referenced is not going to fly.
> 
> If we choose this path, at least put a debug() statement in to warn when
> malloc(0) is called.
> 
> Regards,
> 
> Graeme
> 
> [1] Apparently the way do do it is to reserve the entire first 4kB page and
> mark it as 'not-present' so any access causes a page-fault.

Ok, I don't mean to reopen this can of worms again ... but what're we going to 
do about this patch?

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marex@denx.de> wrote on 2012/10/16 08:31:20:
> 
> Dear Graeme Russ,
> 
> > On 04/03/2012 09:35 AM, Graeme Russ wrote:
> > > Hi Jocke
> > > 
> > > And I really need to check, but I have a sneaking suspicion that as 
the
> > > code currently stands in U-Boot/x86 dereferencing a NULL pointer 
won't
> > > cause an exception. In x86, U-Boot configures all protected mode 
segments
> > > to be 4GB starting at physical address 0x00000000 with no virtual 
address
> > > translation. Accessing physical address 0x00000000 is just as valid 
as
> > > accessing 0x00000001 (or any other address).
> > > 
> > > Now if I set segments to start at 0x00000002 then I can trap a
> > > segmentation fault for accesses to 0x00000000 (NULL) and 0x00000001
> > > (malloc(0) pointer)
> > > 
> > > That will mean that U-Boot cannot ever access those two bytes of 
memory,
> > > but I doubt that I would ever want to. And I will need to set the
> > > segments to base address 0x00000000 before jumping into Linux...
> > 
> > OK, this is not as easy as it sounds. Detecting NULL pointer 
dereferences
> > will involve enabling paging[1] which is something I really do not 
want to
> > do in U-Boot. Flat Protected Mode with a 4GB linear map is perfectly 
fit
> > for purpose, and that is how the Linux kernel expects things to be
> > configured so it will be a major PITA to change.
> > 
> > In short, returning non-NULL from malloc(0) and expecting a CPU 
exception
> > when it is de-referenced is not going to fly.
> > 
> > If we choose this path, at least put a debug() statement in to warn 
when
> > malloc(0) is called.
> > 
> > Regards,
> > 
> > Graeme
> > 
> > [1] Apparently the way do do it is to reserve the entire first 4kB 
page and
> > mark it as 'not-present' so any access causes a page-fault.
> 
> Ok, I don't mean to reopen this can of worms again ... but what're we 
going to 
> do about this patch?

Skip the idea to protect a page, this is too complicated for a boot 
loader. Just
treat malloc(0) as malloc(1) internally.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Joakim Tjernlund,

[...]

> > do about this patch?
> 
> Skip the idea to protect a page, this is too complicated for a boot
> loader. Just
> treat malloc(0) as malloc(1) internally.

I was more interested in knowing if we should drop the patch or what ... ?

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Marek Vasut <marex@denx.de> wrote on 2012/10/16 12:43:08:
> 
> Dear Joakim Tjernlund,
> 
> [...]
> 
> > > do about this patch?
> > 
> > Skip the idea to protect a page, this is too complicated for a boot
> > loader. Just
> > treat malloc(0) as malloc(1) internally.
> 
> I was more interested in knowing if we should drop the patch or what ... 
?

oh, yes I think s?.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Wolfgang Denk]

Dear Marek Vasut,

In message <201210160831.20759.marex@denx.de> you wrote:
> 
> > In short, returning non-NULL from malloc(0) and expecting a CPU exception
> > when it is de-referenced is not going to fly.

We should not expect to have support for any exceptions for any kind
of illegal accesses.  In general, behaviours is undetermined.

> > [1] Apparently the way do do it is to reserve the entire first 4kB page and
> > mark it as 'not-present' so any access causes a page-fault.
> 
> Ok, I don't mean to reopen this can of worms again ... but what're we going to 
> do about this patch?

NAK it.

It is perfectly valid on most systems to dereference a pointer to
address 0 (which in almost all cases looks the same as a NULL
pointer).

Test on ARM (some i.MX31 board):

=> md 0 20
00000000: e59ff00c e59ff018 e59ff018 e59ff018    ................
00000010: e59ff018 a0000000 e59ff014 e59ff014    ................
00000020: 00000090 1fffffd0 1fffffd4 1fffffd8    ................
00000030: 1fffffdc 1fffffe0 1fffffe4 ffffffff    ................
00000040: 79706f43 68676972 63282074 30322029    Copyright (c) 20
00000050: 4d203430 726f746f 20616c6f 2e636e49    04 Motorola Inc.
00000060: 6c6c4120 67697220 20737468 65736572     All rights rese
00000070: 64657672 0000002e ffffffff ffffffff    rved............

Test on PPC (some MPC5200 board):

=> md 0 10
00000000: 60000000 60000000 60000000 2c050000    `...`...`...,...
00000010: 4182001c 429f0005 7d0802a6 3d080000    A...B...}...=...
00000020: 3908ffe8 483a40c1 7fe00008 7c7f1b78    9...H:@.....|..x
00000030: 3b000000 483a2739 48003519 48003465    ;...H:'9H.5.H.4e


I object against patches that will make access to this data impossible
(or even more complicated than it is now).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Don't tell me how hard you work.  Tell me how much you get done.
                                                     -- James J. Ling

[U-Boot] [PATCH] Prevent malloc with size 0

[Graeme Russ]

Hi Wolfgang,

On Tue, Oct 16, 2012 at 9:43 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Marek Vasut,
>
> In message <201210160831.20759.marex@denx.de> you wrote:
>>
>> > In short, returning non-NULL from malloc(0) and expecting a CPU exception
>> > when it is de-referenced is not going to fly.
>
> We should not expect to have support for any exceptions for any kind
> of illegal accesses.  In general, behaviours is undetermined.
>
>> > [1] Apparently the way do do it is to reserve the entire first 4kB page and
>> > mark it as 'not-present' so any access causes a page-fault.
>>
>> Ok, I don't mean to reopen this can of worms again ... but what're we going to
>> do about this patch?
>
> NAK it.

That was my thought

> It is perfectly valid on most systems to dereference a pointer to
> address 0 (which in almost all cases looks the same as a NULL
> pointer).

In an OS environment, it is valid to dereference _physical_ address 0
but not _virtual_ address 0. To achieve this, you need to configure
the MMU accordingly. For x86, this means enabling paging and
configuring the physical/virtual address map...

> I object against patches that will make access to this data impossible
> (or even more complicated than it is now).

Exactly - way too complicated for the (questionable) benefit it provides.

Regards,

Graeme

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Sunday 01 April 2012 18:40:05 Graeme Russ wrote:
>         if ((long)bytes == 0) {
>                 DEBUG("Warning: malloc of zero block size\n");
>                 bytes = 1;
>         } else if ((long)bytes < 0) {
>                 DEBUG("Error: malloc of negative block size\n");
>                 return 0;
>         }

this should be (ssize_t) casts, not (long)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120401/205427d2/attachment.pgp>

[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Mike Frysinger,

> On Sunday 01 April 2012 18:40:05 Graeme Russ wrote:
> >         if ((long)bytes == 0) {
> >         
> >                 DEBUG("Warning: malloc of zero block size\n");
> >                 bytes = 1;
> >         
> >         } else if ((long)bytes < 0) {
> >         
> >                 DEBUG("Error: malloc of negative block size\n");
> >                 return 0;
> >         
> >         }
> 
> this should be (ssize_t) casts, not (long)

This ain't the point of this, but you're right.

> -mike

Best regards,
Marek Vasut

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Mike Frysinger <vapier@gentoo.org> wrote on 2010/10/21 21:51:53:
> 
> On Thursday, October 21, 2010 07:45:10 Joakim Tjernlund wrote:
> > Wolfgang Denk wrote on 2010/10/21 13:32:54:
> > > Joakim Tjernlund you wrote:
> > > > > -  if ((long)bytes < 0) return 0;
> > > > > +  if ((long)bytes <= 0) return 0;
> > > > 
> > > > I think you should return some impossible ptr value =! NULL
> > > > Size 0 not really an error.
> > > 
> > > It is legal for malloc() to return NULL in case of size==0,
> > > and for the sake of simplicity I recommend we do just that.
> > 
> > Yes, but not very useful. Glibc does not return NULL
> 
> it is useful for malloc(0) == NULL.  the glibc behavior is downright 
> obnoxious.  we disable this for uClibc and dont see problems.  if 
anything, we 
> catch accidental programming mistakes which then get fixed.

There is a value in having the possibility to express a
0 bytes data set. Consider this simple example:
An app read lines from a file and mallocs each line read and builds an
array with malloced pointers. The last entry is a NULL ptr to
signal EOF. This breaks down for empty lines if malloc(0)
returns NULL. 

Not to mention error handling, as I recall, a malloc(0) that returns NULL
does not set errno which screws error handling. One have to bend over
just to cope with this.
 
> 
> why exactly do you want malloc(0) to return valid memory ?  i would 
rather 
> have u-boot return an error.

Ideally it should return a ptr to invalid memory so you get a SEGV if you
try to defer the ptr but I take anything over a NULL ptr.

 Jocke

[U-Boot] [PATCH] Prevent malloc with size 0

[Reinhard Meyer]

Dear Joakim Tjernlund,
> Mike Frysinger <vapier@gentoo.org> wrote on 2010/10/21 21:51:53:
>> On Thursday, October 21, 2010 07:45:10 Joakim Tjernlund wrote:
>>> Wolfgang Denk wrote on 2010/10/21 13:32:54:
>>>> Joakim Tjernlund you wrote:
>>>>>> -  if ((long)bytes < 0) return 0;
>>>>>> +  if ((long)bytes <= 0) return 0;
>>>>> I think you should return some impossible ptr value =! NULL
>>>>> Size 0 not really an error.
>>>> It is legal for malloc() to return NULL in case of size==0,
>>>> and for the sake of simplicity I recommend we do just that.
>>> Yes, but not very useful. Glibc does not return NULL
>> it is useful for malloc(0) == NULL.  the glibc behavior is downright 
>> obnoxious.  we disable this for uClibc and dont see problems.  if 
> anything, we 
>> catch accidental programming mistakes which then get fixed.

My five cents:

> There is a value in having the possibility to express a
> 0 bytes data set. Consider this simple example:
> An app read lines from a file and mallocs each line read and builds an
> array with malloced pointers. The last entry is a NULL ptr to
> signal EOF. This breaks down for empty lines if malloc(0)
> returns NULL. 

Your example is in the right way, but a bit flawed in its simplicity.
Even empty lines need some form of information that they are of length zero,
be it a 0x00 in the memory line itself (requiring malloc(length+1)) or that same
information in a variable somewhere else:
struct line {
	byte *buf;
	int length;
} lines[...];

As an (undercover) Mathematician:
Out of principle I would say that malloc(0) should return a non-NULL
pointer of an area where exactly 0 bytes may be used. And, of course,
free() of that area shall not fail or crash the system.

> Not to mention error handling, as I recall, a malloc(0) that returns NULL
> does not set errno which screws error handling. One have to bend over
> just to cope with this.

>> why exactly do you want malloc(0) to return valid memory ?  i would 
> rather 
>> have u-boot return an error.

In the case of u-boot, where a driver or whatever should never really need
to allocate zero memory, such a programming error should be made obvious by
at least a warning message.
 
> Ideally it should return a ptr to invalid memory so you get a SEGV if you
> try to defer the ptr but I take anything over a NULL ptr.

Makes sense only if any access outside of any allocated memory would behave
the same, otherwise this is a special case again.

Best Regards,
Reinhard

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Reinhard Meyer <u-boot@emk-elektronik.de> wrote on 2010/10/22 09:18:02:
> 
> Dear Joakim Tjernlund,
> > Mike Frysinger <vapier@gentoo.org> wrote on 2010/10/21 21:51:53:
> >> On Thursday, October 21, 2010 07:45:10 Joakim Tjernlund wrote:
> >>> Wolfgang Denk wrote on 2010/10/21 13:32:54:
> >>>> Joakim Tjernlund you wrote:
> >>>>>> -  if ((long)bytes < 0) return 0;
> >>>>>> +  if ((long)bytes <= 0) return 0;
> >>>>> I think you should return some impossible ptr value =! NULL
> >>>>> Size 0 not really an error.
> >>>> It is legal for malloc() to return NULL in case of size==0,
> >>>> and for the sake of simplicity I recommend we do just that.
> >>> Yes, but not very useful. Glibc does not return NULL
> >> it is useful for malloc(0) == NULL.  the glibc behavior is downright 
> >> obnoxious.  we disable this for uClibc and dont see problems.  if 
> > anything, we 
> >> catch accidental programming mistakes which then get fixed.
> 
> My five cents:
> 
> > There is a value in having the possibility to express a
> > 0 bytes data set. Consider this simple example:
> > An app read lines from a file and mallocs each line read and builds an
> > array with malloced pointers. The last entry is a NULL ptr to
> > signal EOF. This breaks down for empty lines if malloc(0)
> > returns NULL. 
> 
> Your example is in the right way, but a bit flawed in its simplicity.
> Even empty lines need some form of information that they are of length 
zero,
> be it a 0x00 in the memory line itself (requiring malloc(length+1)) or 
that same
> information in a variable somewhere else:
> struct line {
>    byte *buf;
>    int length;
> } lines[...];

Thanks, this is better.

> As an (undercover) Mathematician:
> Out of principle I would say that malloc(0) should return a non-NULL
> pointer of an area where exactly 0 bytes may be used. And, of course,
> free() of that area shall not fail or crash the system.
> 
> > Not to mention error handling, as I recall, a malloc(0) that returns 
NULL
> > does not set errno which screws error handling. One have to bend over
> > just to cope with this.
> 
> >> why exactly do you want malloc(0) to return valid memory ?  i would 
> > rather 
> >> have u-boot return an error.
> 
> In the case of u-boot, where a driver or whatever should never really 
need
> to allocate zero memory, such a programming error should be made obvious 
by
> at least a warning message.

Right, a driver probably doesn't need malloc(0) != NULL but some
command may.

You could have a DEBUG printout for 0 if you like though.

> 
> > Ideally it should return a ptr to invalid memory so you get a SEGV if 
you
> > try to defer the ptr but I take anything over a NULL ptr.
> 
> Makes sense only if any access outside of any allocated memory would 
behave
> the same, otherwise this is a special case again.

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Friday, October 22, 2010 02:10:16 Joakim Tjernlund wrote:
> Mike Frysinger wrote on 2010/10/21 21:51:53:
> > it is useful for malloc(0) == NULL.  the glibc behavior is downright
> > obnoxious.  we disable this for uClibc and dont see problems.  if
> > anything, we catch accidental programming mistakes which then get fixed.
> 
> There is a value in having the possibility to express a
> 0 bytes data set. Consider this simple example:
> An app read lines from a file and mallocs each line read and builds an
> array with malloced pointers. The last entry is a NULL ptr to
> signal EOF. This breaks down for empty lines if malloc(0)
> returns NULL.

i dont think so ... C strings are always at least one byte, so an empty line 
would be malloc(1)

> Not to mention error handling, as I recall, a malloc(0) that returns NULL
> does not set errno which screws error handling. One have to bend over
> just to cope with this.

that depends on your implementation.  in u-boot, there really is no "errno" 
concept.  but i dont see how this matters as i dont see any example in u-boot 
context where malloc(0) is a problem.

> > why exactly do you want malloc(0) to return valid memory ?  i would
> > rather have u-boot return an error.
> 
> Ideally it should return a ptr to invalid memory so you get a SEGV if you
> try to defer the ptr but I take anything over a NULL ptr.

the concept of an invalid pointer is pretty arch-specific.  and it changes the 
semantics of what the vast majority of coders (and their code) out there 
expect -- NULL means error while non-NULL means success.

a better question might be "why isnt a NULL pointer on your platform an 
invalid pointer" ?  ive added a simple CONFIG define for Blackfin users to do 
just that -- make access to the low 1KiB of memory generate an exception.  it 
does so with pretty much 0 runtime overhead.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101022/ef1945ae/attachment.pgp 

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Mike Frysinger <vapier@gentoo.org> wrote on 2010/10/22 09:20:22:
> 
> On Friday, October 22, 2010 02:10:16 Joakim Tjernlund wrote:
> > Mike Frysinger wrote on 2010/10/21 21:51:53:
> > > it is useful for malloc(0) == NULL.  the glibc behavior is downright
> > > obnoxious.  we disable this for uClibc and dont see problems.  if
> > > anything, we catch accidental programming mistakes which then get 
fixed.
> > 
> > There is a value in having the possibility to express a
> > 0 bytes data set. Consider this simple example:
> > An app read lines from a file and mallocs each line read and builds an
> > array with malloced pointers. The last entry is a NULL ptr to
> > signal EOF. This breaks down for empty lines if malloc(0)
> > returns NULL.
> 
> i dont think so ... C strings are always at least one byte, so an empty 
line 
> would be malloc(1)

Yes, the example was a little flawed but the was the best I could come
up with on short notice. I am sure my point was understood anyway.

> 
> > Not to mention error handling, as I recall, a malloc(0) that returns 
NULL
> > does not set errno which screws error handling. One have to bend over
> > just to cope with this.
> 
> that depends on your implementation.  in u-boot, there really is no 
"errno"

Yes, and that and that is even worse. How do you tell if you are out of 
memory
or not? Checking size == 0 after the fact? Then you could do that
before calling malloc in the first place.
 
> concept.  but i dont see how this matters as i dont see any example in 
u-boot 
> context where malloc(0) is a problem.

This is bigger than u-boot and you just because there isn't someone using 
it
today(how can you when it corrupts memory), that might change in the 
future.

> 
> > > why exactly do you want malloc(0) to return valid memory ?  i would
> > > rather have u-boot return an error.
> > 
> > Ideally it should return a ptr to invalid memory so you get a SEGV if 
you
> > try to defer the ptr but I take anything over a NULL ptr.
> 
> the concept of an invalid pointer is pretty arch-specific.  and it 
changes the 
> semantics of what the vast majority of coders (and their code) out there 

> expect -- NULL means error while non-NULL means success.

hardly as glibc returns a non NULL ptr. Changing this would probably
break apps, making them think they are out of memory.

non null still what it is supposed to mean, you successfully allocated
0 bytes.

> a better question might be "why isnt a NULL pointer on your platform an 
> invalid pointer" ?  ive added a simple CONFIG define for Blackfin users 
to do 
> just that -- make access to the low 1KiB of memory generate an 
exception.  it 
> does so with pretty much 0 runtime overhead.

You lost me here. How does this relate to the issue before us?

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Friday, October 22, 2010 03:37:43 Joakim Tjernlund wrote:
> Mike Frysinger wrote on 2010/10/22 09:20:22:
> > On Friday, October 22, 2010 02:10:16 Joakim Tjernlund wrote:
> > > does not set errno which screws error handling. One have to bend over
> > > just to cope with this.
> > 
> > that depends on your implementation.  in u-boot, there really is no
> > "errno"
> 
> Yes, and that and that is even worse. How do you tell if you are out of
> memory or not? Checking size == 0 after the fact? Then you could do that
> before calling malloc in the first place.

i still dont see any real world (or even theoretical) need for malloc(0).  so 
the issue of error checking is irrelevant until you can come up with one.

> > the concept of an invalid pointer is pretty arch-specific.  and it changes
> > the semantics of what the vast majority of coders (and their code) out
> > there expect -- NULL means error while non-NULL means success.
> 
> hardly as glibc returns a non NULL ptr. Changing this would probably
> break apps, making them think they are out of memory.

funny because i have no problem running software on top of uClibc.  and it 
does do malloc(0) == NULL.

just visit http://wh0rd.org/ and marvel at all the software running perfectly 
fine on uClibc.  apache/openssh/openvpn/php/gd/git/svn/... 

> non null still what it is supposed to mean, you successfully allocated
> 0 bytes.

POSIX clearly labels it "implementation defined" which means "no sane portable 
programmer may rely on its behavior"

> > a better question might be "why isnt a NULL pointer on your platform an
> > invalid pointer" ?  ive added a simple CONFIG define for Blackfin users
> > to do just that -- make access to the low 1KiB of memory generate an
> > exception.  it does so with pretty much 0 runtime overhead.
> 
> You lost me here. How does this relate to the issue before us?

you're asking for an invalid pointer.  the presumption is that you want it so 
that attempts to access it will fail.  if that isnt the case, i have no idea 
why you want malloc(0) to return an "invalid pointer" that isnt NULL.

btw, could you please fix your mailer.  it constantly destroys quoted context.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101022/483fb45d/attachment.pgp 

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Mike Frysinger <vapier@gentoo.org> wrote on 2010/10/22 09:55:49:
> 
> On Friday, October 22, 2010 03:37:43 Joakim Tjernlund wrote:
> > Mike Frysinger wrote on 2010/10/22 09:20:22:
> > > On Friday, October 22, 2010 02:10:16 Joakim Tjernlund wrote:
> > > > does not set errno which screws error handling. One have to bend 
over
> > > > just to cope with this.
> > > 
> > > that depends on your implementation.  in u-boot, there really is no
> > > "errno"
> > 
> > Yes, and that and that is even worse. How do you tell if you are out 
of
> > memory or not? Checking size == 0 after the fact? Then you could do 
that
> > before calling malloc in the first place.
> 
> i still dont see any real world (or even theoretical) need for 
malloc(0).  so 
> the issue of error checking is irrelevant until you can come up with 
one.

Don't you want to know if the app screwed up or if the system is out
of memory?

The only upside to malloc(0) == NULL I can see is an extra check
for apps for which size == 0 is an error to the app too(something the app
should have checked itself long before calling malloc). 

> 
> > > the concept of an invalid pointer is pretty arch-specific.  and it 
changes
> > > the semantics of what the vast majority of coders (and their code) 
out
> > > there expect -- NULL means error while non-NULL means success.
> > 
> > hardly as glibc returns a non NULL ptr. Changing this would probably
> > break apps, making them think they are out of memory.
> 
> funny because i have no problem running software on top of uClibc.  and 
it 
> does do malloc(0) == NULL.
> 
> just visit http://wh0rd.org/ and marvel at all the software running 
perfectly 
> fine on uClibc.  apache/openssh/openvpn/php/gd/git/svn/... 
> 
> > non null still what it is supposed to mean, you successfully allocated
> > 0 bytes.
> 
> POSIX clearly labels it "implementation defined" which means "no sane 
portable 
> programmer may rely on its behavior"

Yes and that is probably why you don't see any problems. Apps has learned 
this
the hard way and coded accordingly to avoid problems. That doesn't mean
anything w.r.t what is the better behaviour from malloc(0)

> 
> > > a better question might be "why isnt a NULL pointer on your platform 
an
> > > invalid pointer" ?  ive added a simple CONFIG define for Blackfin 
users
> > > to do just that -- make access to the low 1KiB of memory generate an
> > > exception.  it does so with pretty much 0 runtime overhead.
> > 
> > You lost me here. How does this relate to the issue before us?
> 
> you're asking for an invalid pointer.  the presumption is that you want 
it so 
> that attempts to access it will fail.  if that isnt the case, i have no 
idea 
> why you want malloc(0) to return an "invalid pointer" that isnt NULL.

ah, I guess asking for invalid ptrs in u-boot is too much for a boot app 
so
lets forget about that.

> 
> btw, could you please fix your mailer.  it constantly destroys quoted 
context.

I wish, we use Lotus/Domino here nowdays. I have sent numerous reports
to IBM about their crappy email support. I have been promised they
will fix some of them to the next release but I am not holding my breath 
:(

hmm, "destroys quoted context", I think that is a new one. What
exactly is destroyed?

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Friday, October 22, 2010 04:34:52 Joakim Tjernlund wrote:
> Don't you want to know if the app screwed up or if the system is out
> of memory?
> 
> The only upside to malloc(0) == NULL I can see is an extra check
> for apps for which size == 0 is an error to the app too(something the app
> should have checked itself long before calling malloc).

if an app is doing malloc(0), it is broken.  i'm really not worried at all 
about detecting OOM state.  my real world experience thus far has shown no 
actual problems with this behavior.  so unless you have some actual examples 
where this behavior "harms" u-boot, i say we merge the OP's patch.

> hmm, "destroys quoted context", I think that is a new one. What
> exactly is destroyed?

it seems it fails to rewrap quoted lines and then when other people reply, 
their clients arent able to recover thus screwing it up even more.  pick any 
msg from you in this thread.  like this one:
	http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/87174/focus=87290
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101022/5842be53/attachment.pgp 

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Mike Frysinger <vapier@gentoo.org> wrote on 2010/10/22 17:18:05:
>
> On Friday, October 22, 2010 04:34:52 Joakim Tjernlund wrote:
> > Don't you want to know if the app screwed up or if the system is out
> > of memory?
> >
> > The only upside to malloc(0) == NULL I can see is an extra check
> > for apps for which size == 0 is an error to the app too(something the app
> > should have checked itself long before calling malloc).
>
> if an app is doing malloc(0), it is broken.  i'm really not worried at all

In your opinion, not mine and not POSIX either. I outlined uses for malloc(0) earlier.

> about detecting OOM state.  my real world experience thus far has shown no
> actual problems with this behavior.  so unless you have some actual examples
> where this behavior "harms" u-boot, i say we merge the OP's patch.

Since you think an app is broken if it does malloc(0), it should not matter
what malloc(0) returns to you. You only gain an extra error check for the broken app
at the expense of ease of use and sane error checking in general.
I prefer not to make it easier for broken apps when it hurts elsewhere.

>
> > hmm, "destroys quoted context", I think that is a new one. What
> > exactly is destroyed?
>
> it seems it fails to rewrap quoted lines and then when other people reply,
> their clients arent able to recover thus screwing it up even more.  pick any
> msg from you in this thread.  like this one:
>    http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/87174/focus=87290

Ah, IT changed some settings globally. I changed it back. I hope this reply
is better.

[U-Boot] [PATCH] Prevent malloc with size 0

[Mike Frysinger]

On Friday, October 22, 2010 12:40:07 Joakim Tjernlund wrote:
> Mike Frysinger wrote on 2010/10/22 17:18:05:
> > On Friday, October 22, 2010 04:34:52 Joakim Tjernlund wrote:
> > > Don't you want to know if the app screwed up or if the system is out
> > > of memory?
> > > 
> > > The only upside to malloc(0) == NULL I can see is an extra check
> > > for apps for which size == 0 is an error to the app too(something the
> > > app should have checked itself long before calling malloc).
> > 
> > if an app is doing malloc(0), it is broken.  i'm really not worried at
> > all
> 
> In your opinion, not mine and not POSIX either. I outlined uses for
> malloc(0) earlier.

i havent seen any realistic usage examples.  the only ones you quoted could 
easily be solved in a different way assuming they were even affected by 
malloc(0).

> > about detecting OOM state.  my real world experience thus far has shown
> > no actual problems with this behavior.  so unless you have some actual
> > examples where this behavior "harms" u-boot, i say we merge the OP's
> > patch.
> 
> Since you think an app is broken if it does malloc(0), it should not matter
> what malloc(0) returns to you. You only gain an extra error check for the
> broken app at the expense of ease of use and sane error checking in
> general.

more like you notice that the code is broken right away.  the few times i have 
seen a malloc(0), it was do to errors elsewhere in the code and once those 
were fixed, malloc() wasnt called with a size 0.

> I prefer not to make it easier for broken apps when it hurts elsewhere.

funny, i can use the same logic to ban malloc(0).  it makes you notice & fix 
errors without penalties.

> Ah, IT changed some settings globally. I changed it back. I hope this reply
> is better.

yes, this is much better.  thanks !
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101022/0b9082f7/attachment.pgp 

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Mike Frysinger <vapier@gentoo.org> wrote on 2010/10/22 19:06:14:
>
> On Friday, October 22, 2010 12:40:07 Joakim Tjernlund wrote:
> > Mike Frysinger wrote on 2010/10/22 17:18:05:
> > > On Friday, October 22, 2010 04:34:52 Joakim Tjernlund wrote:
> > > > Don't you want to know if the app screwed up or if the system is out
> > > > of memory?
> > > >
> > > > The only upside to malloc(0) == NULL I can see is an extra check
> > > > for apps for which size == 0 is an error to the app too(something the
> > > > app should have checked itself long before calling malloc).
> > >
> > > if an app is doing malloc(0), it is broken.  i'm really not worried at
> > > all
> >
> > In your opinion, not mine and not POSIX either. I outlined uses for
> > malloc(0) earlier.
>
> i havent seen any realistic usage examples.  the only ones you quoted could
> easily be solved in a different way assuming they were even affected by
> malloc(0).

Scott was kind to provide an example(thanks Scott).

I never said it wasn't possible to workaround malloc(0) == NULL, I say that
you should not have to.

>
> > > about detecting OOM state.  my real world experience thus far has shown
> > > no actual problems with this behavior.  so unless you have some actual
> > > examples where this behavior "harms" u-boot, i say we merge the OP's
> > > patch.
> >
> > Since you think an app is broken if it does malloc(0), it should not matter
> > what malloc(0) returns to you. You only gain an extra error check for the
> > broken app at the expense of ease of use and sane error checking in
> > general.
>
> more like you notice that the code is broken right away.  the few times i have
> seen a malloc(0), it was do to errors elsewhere in the code and once those
> were fixed, malloc() wasnt called with a size 0.
>
> > I prefer not to make it easier for broken apps when it hurts elsewhere.
>
> funny, i can use the same logic to ban malloc(0).  it makes you notice & fix
> errors without penalties.

Not quite, first you should show that there aren't any
uses for malloc(0) != NULL as I have done

Actually POSIX doesn't say that malloc(0) is an error:
  Upon successful completion with size not equal to 0, malloc() shall return a pointer to the allocated space.
  If size is 0, either a null pointer or a unique pointer that can be successfully passed to free() shall be returned.
  Otherwise, it shall return a null pointer and set errno to indicate the error.

it only say that you can chose to return NULL or an unique ptr.

I don't think it is any value debating this any more though. You have
your view and I got mine.

>
> > Ah, IT changed some settings globally. I changed it back. I hope this reply
> > is better.
>
> yes, this is much better.  thanks !

Great, I really hate it when replies gets mangled too

[U-Boot] [PATCH] Prevent malloc with size 0

[Scott Wood]

On Fri, 22 Oct 2010 03:55:49 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> On Friday, October 22, 2010 03:37:43 Joakim Tjernlund wrote:
> > Mike Frysinger wrote on 2010/10/22 09:20:22:
> > > On Friday, October 22, 2010 02:10:16 Joakim Tjernlund wrote:
> > > > does not set errno which screws error handling. One have to bend over
> > > > just to cope with this.
> > > 
> > > that depends on your implementation.  in u-boot, there really is no
> > > "errno"
> > 
> > Yes, and that and that is even worse. How do you tell if you are out of
> > memory or not? Checking size == 0 after the fact? Then you could do that
> > before calling malloc in the first place.
> 
> i still dont see any real world (or even theoretical) need for malloc(0).  so 
> the issue of error checking is irrelevant until you can come up with one.

Here's a (non-U-Boot) example from some code that unflattens a device
tree into a live tree representation:

	prop->len = fdt32_to_cpu(fdtprop->len);
	fdtprop = fdt_offset_ptr(fdt, offset, sizeof(*fdtprop) + prop->len);
	if (!fdtprop) {
		ret = -FDT_ERR_TRUNCATED;
		goto err;
	}

	prop->data = malloc(prop->len);
	if (!prop->data)
		goto nomem;

You couldn't do this in portable code, since malloc(0) is allowed to return
NULL, and it wouldn't be hard to work around it by checking prop->len for
zero.  But it is a use case where malloc(0) returning non-NULL is
convenient.

I don't think Joakim's suggestion of a single "impossible_ptr" is compliant,
though -- it's supposed to be either NULL or a *unique* pointer.

-Scott

[U-Boot] [PATCH] Prevent malloc with size 0

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2010/10/22 19:36:33:
>
> On Fri, 22 Oct 2010 03:55:49 -0400
> Mike Frysinger <vapier@gentoo.org> wrote:
>
> > On Friday, October 22, 2010 03:37:43 Joakim Tjernlund wrote:
> > > Mike Frysinger wrote on 2010/10/22 09:20:22:
> > > > On Friday, October 22, 2010 02:10:16 Joakim Tjernlund wrote:
>
> You couldn't do this in portable code, since malloc(0) is allowed to return
> NULL, and it wouldn't be hard to work around it by checking prop->len for
> zero.  But it is a use case where malloc(0) returning non-NULL is
> convenient.
>
> I don't think Joakim's suggestion of a single "impossible_ptr" is compliant,
> though -- it's supposed to be either NULL or a *unique* pointer.

Somewhat debatable but as long it is != NULL it will do.

Would be interesting to know what other libc's/OS do.

 Jocke