malloc: compare pointer to NULL not to 0.
PATCH 1/1 malloc: compare pointer to NULL not to 0.
victim is defined as:
typedef struct malloc_chunk* mchunkptr;
mchunkptr victim;
So victim should be compared to NULL and not to 0.
Problem identified with Coccinelle.
--- common/dlmalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
common/dlmalloc.c +1 -1
@@ -1461,7 +1461,7 @@ Void_t* mALLOc(bytes) size_t bytes;#if HAVE_MMAP /* If big and would otherwise need to extend, try to use mmap instead */ if ((unsigned long )nb >= (unsigned long )mmap_threshold && - (victim =mmap_chunk (nb)) !=0 ) + (victim =mmap_chunk (nb)) !=NULL )return chunk2mem (victim);#endif
Signature · 1 line
2.14.2
PATCH v2 1/1 malloc: don't compare pointers to 0
0 is not a pointer. So do not compare pointers to 0.
Do not return 0 from functions with a pointer return type.
Problem identified with Coccinelle.
---
v2
Fix more occurences of the problem.
v1
[PATCH 1/1] malloc: compare pointer to NULL not to 0. https://patchwork.ozlabs.org/patch/836850/ --- common/dlmalloc.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
common/dlmalloc.c +9 -5
@@ -149,7 +149,7 @@ gAllocatedSize)) { new_address =findRegion (new_address, new_size); -if (new_address ==0 ) +if (!new_address)return (void *)-1 ; gAddressBase = gNextAddress =
@@ -175,7 +175,7 @@ gAllocatedSize)) (size + gNextAddress -AlignPage (gNextAddress)), MEM_COMMIT, PAGE_READWRITE); -if (res ==0 ) +if (!res)return (void *)-1 ; } tmp = (void *)gNextAddress;
@@ -1461,7 +1461,7 @@ Void_t* mALLOc(bytes) size_t bytes;#if HAVE_MMAP /* If big and would otherwise need to extend, try to use mmap instead */ if ((unsigned long )nb >= (unsigned long )mmap_threshold && - (victim =mmap_chunk (nb)) !=0 ) + (victim =mmap_chunk (nb)))return chunk2mem (victim);#endif
@@ -1671,7 +1671,10 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes;mchunkptr fwd;/* misc temp for linking */ #ifdef REALLOC_ZERO_BYTES_FREES -if (bytes ==0 ) {fREe (oldmem);return 0 ; } +if (!bytes) { +fREe (oldmem); +return NULL ; + }#endif if ((long )bytes <0 )return NULL ;
@@ -1703,7 +1706,8 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes;if (oldsize -SIZE_SZ >= nb)return oldmem;/* do nothing */ /* Must alloc, copy, free. */ newmem =mALLOc (bytes); -if (newmem ==0 )return 0 ;/* propagate failure */ +if (!newmem) +return NULL ;/* propagate failure */ MALLOC_COPY (newmem, oldmem, oldsize -2 *SIZE_SZ );munmap_chunk (oldp);return newmem;
Signature · 1 line
2.14.2
PATCH v2 1/1 malloc: don't compare pointers to 0
Hello Heinrich,
Quoted 6 lines · Heinrich Schuchardt
0 is not a pointer. So do not compare pointers to 0.
Do not return 0 from functions with a pointer return type.
Problem identified with Coccinelle.
Perhaps you can elaborate on what problem you are solving? As far as I know it is only a style issue...
Regards,
Jeroen
PATCH v2 1/1 malloc: don't compare pointers to 0
Quoted 5 lines · Jeroen Hofstee
Hello Heinrich,
Quoted 2 lines · Heinrich Schuchardt
0 is not a pointer. So do not compare pointers to 0.
Quoted 7 lines
Quoted 4 lines
Do not return 0 from functions with a pointer return type.
Problem identified with Coccinelle.
Perhaps you can elaborate on what problem you are solving? As far as I know it is only a style issue...
Yes the file is written quite undecently in many aspects.
Regards
Heirnich
PATCH v2 1/1 malloc: don't compare pointers to 0
In reply to Heinrich SchuchardtDear Heinrich,
Quoted 1 line · In message <[email protected]> you wrote:
0 is not a pointer. So do not compare pointers to 0.
Who says so? 0 can be the value of a pointer to a valid, existing address. For example a large number of Power Architecture systems map the RAM beginning at physical address 0, so if you want to wrtite to the very first byte in memory your pointer value is 0.
Quoted 2 lines
Do not return 0 from functions with a pointer return type.
Why? It could be a valid value - or not.
common/dlmalloc.c gAllocatedSize)) · line 152
Quoted 2 lines
common/dlmalloc.c +1
-1-if (new_address ==0 ) +if (!new_address)
In which way is "new_address == 0" and "!new_address" different?
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
He'd heard her use that sweet, innocent tone of voice before. It
meant that, pretty soon, there was going to be trouble.
- Terry Pratchett, _Truckers_PATCH v2 1/1 malloc: don't compare pointers to 0
Quoted 9 lines · Wolfgang Denk
Dear Heinrich,
Quoted 1 line · In message <[email protected]> you wrote:
0 is not a pointer. So do not compare pointers to 0.
Who says so? 0 can be the value of a pointer to a valid, existing address. For example a large number of Power Architecture systems map the RAM beginning at physical address 0, so if you want to wrtite to the very first byte in memory your pointer value is 0.
I think you just proved the point here. One can write clever code knowing that on architecture X we can do trick Y (and hey, for well commented assembly, go reasonably nuts!0. But in C and as we get more and better and easier to use static analysis tools we write clear and consistent code.
Quoted 9 lines
Quoted 2 lines
Do not return 0 from functions with a pointer return type.
Why? It could be a valid value - or not.
common/dlmalloc.c gAllocatedSize)) · line 152
Quoted 2 lines
common/dlmalloc.c +1
-1-if (new_address ==0 ) +if (!new_address)In which way is "
new_address== 0" and "!new_address" different?
And yes, it's not going to change the generated code.
Really, the best argument for not changing anything in this file is that it brings us further from upstream, and we've neither thrown that notion out the window nor frequently re-synced with it.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171113/5ee5d169/attachment.sig>
PATCH v2 1/1 malloc: don't compare pointers to 0
In reply to Heinrich SchuchardtQuoted 14 lines · Heinrich Schuchardt
0 is not a pointer. So do not compare pointers to 0.
Do not return 0 from functions with a pointer return type.
Problem identified with Coccinelle.
Signed-off-by Heinrich Schuchardt <[email protected]>---
v2
Fix more occurences of the problem.
v1
[PATCH 1/1] malloc: compare pointer to NULL not to 0.
https://patchwork.ozlabs.org/patch/836850/
Quoted 4 lines
--- common/dlmalloc.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
PATCH v2 1/1 malloc: don't compare pointers to 0
In reply to Heinrich SchuchardtQuoted 9 lines · Heinrich Schuchardt
0 is not a pointer. So do not compare pointers to 0.
Do not return 0 from functions with a pointer return type.
Problem identified with Coccinelle.
Signed-off-by Heinrich Schuchardt <[email protected]>Reviewed-by Bin Meng <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171121/d3ede1f5/attachment.sig>