| Message ID | 4CC006B1.8000905@intracomdefense.com |
|---|---|
| State | Rejected |
| Delegated to: | Marek Vasut |
| Headers | show |
> > 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;
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
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
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
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
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
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
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.
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
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
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
> > 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
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
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
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
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
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,
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> > 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
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
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
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
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
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
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
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
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
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
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. From Scotts example we already know there is kernel code that relies on malloc(0) not returning NULL. 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). Jocke
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
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
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
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
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
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
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
vapierfilter@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.
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
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
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.
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
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
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
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
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
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
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; */
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(-) -- 1.6.4.4