Message ID | 5b13bf9e-b30c-4405-b2aa-cfa2b3e4c618@BAMAIL02.ba.imgtec.org |
---|---|
State | New |
Headers | show |
On 12/10/2014 06:13 PM, Steve Ellcey wrote: > Here is a fix for another warning (now error) found in my MIPS build. > > malloc.c: In function '__posix_memalign': > malloc.c:4976:50: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses] > || !powerof2 (alignment / sizeof (void *)) != 0 > ^ > cc1: all warnings being treated as errors > > > The fix is to remove the '!= 0' comparision since it is redundant. Removing '!= 0' creates a boolean coersion from the int type and we avoid this in glibc because it is clearer to say exactly what you mean e.g. != 0 or == 0. Is not the right fix to add a bracket to clarify the `!` is applied to the left side only e.g. "|| (!powerof2 (alignment / sizeof (void *))) != 0"? > OK to checkin? > > Steve Ellcey > sellcey@imgtec.com > > 2014-12-10 Steve Ellcey <sellcey@imgtec.com> > > * malloc/malloc.c: Fix powerof2 check. > > > diff --git a/malloc/malloc.c b/malloc/malloc.c > index 6bfb859..cb91b97 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -4973,7 +4973,7 @@ __posix_memalign (void **memptr, size_t alignment, size_t size) > /* Test whether the SIZE argument is valid. It must be a power of > two multiple of sizeof (void *). */ > if (alignment % sizeof (void *) != 0 > - || !powerof2 (alignment / sizeof (void *)) != 0 > + || !powerof2 (alignment / sizeof (void *)) > || alignment == 0) > return EINVAL; > > c.
On 12/10/2014 08:13 PM, Carlos O'Donell wrote: > On 12/10/2014 06:13 PM, Steve Ellcey wrote: >> Here is a fix for another warning (now error) found in my MIPS build. >> >> malloc.c: In function '__posix_memalign': >> malloc.c:4976:50: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses] >> || !powerof2 (alignment / sizeof (void *)) != 0 >> ^ >> cc1: all warnings being treated as errors >> >> >> The fix is to remove the '!= 0' comparision since it is redundant. > > Removing '!= 0' creates a boolean coersion from the int type and we avoid > this in glibc because it is clearer to say exactly what you mean e.g. != 0 or == 0. > > Is not the right fix to add a bracket to clarify the `!` is applied to the left > side only e.g. "|| (!powerof2 (alignment / sizeof (void *))) != 0"? What I said is wrong, I see powerof2 returns a boolean. >> OK to checkin? OK. Cheers, Carlos.
On Wed, 10 Dec 2014, Carlos O'Donell wrote: > On 12/10/2014 06:13 PM, Steve Ellcey wrote: > > Here is a fix for another warning (now error) found in my MIPS build. > > > > malloc.c: In function '__posix_memalign': > > malloc.c:4976:50: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses] > > || !powerof2 (alignment / sizeof (void *)) != 0 > > ^ > > cc1: all warnings being treated as errors > > > > > > The fix is to remove the '!= 0' comparision since it is redundant. > > Removing '!= 0' creates a boolean coersion from the int type and we avoid > this in glibc because it is clearer to say exactly what you mean e.g. != 0 or == 0. > > Is not the right fix to add a bracket to clarify the `!` is applied to the left > side only e.g. "|| (!powerof2 (alignment / sizeof (void *))) != 0"? The result of "!" is boolean; no implicit boolean conversion is involved (unless you do "!non_boolean", but then you should write "non_boolean == 0" not "(!non_boolean) != 0").
diff --git a/malloc/malloc.c b/malloc/malloc.c index 6bfb859..cb91b97 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -4973,7 +4973,7 @@ __posix_memalign (void **memptr, size_t alignment, size_t size) /* Test whether the SIZE argument is valid. It must be a power of two multiple of sizeof (void *). */ if (alignment % sizeof (void *) != 0 - || !powerof2 (alignment / sizeof (void *)) != 0 + || !powerof2 (alignment / sizeof (void *)) || alignment == 0) return EINVAL;