[MIPS] Fix warning from malloc/malloc.c
diff mbox

Message ID 5b13bf9e-b30c-4405-b2aa-cfa2b3e4c618@BAMAIL02.ba.imgtec.org
State New
Headers show

Commit Message

Steve Ellcey Dec. 10, 2014, 11:13 p.m. UTC
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.

OK to checkin?

Steve Ellcey
sellcey@imgtec.com

2014-12-10  Steve Ellcey  <sellcey@imgtec.com>

	* malloc/malloc.c: Fix powerof2 check.

Comments

Carlos O'Donell Dec. 11, 2014, 1:13 a.m. UTC | #1
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.
Carlos O'Donell Dec. 11, 2014, 1:18 a.m. UTC | #2
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.
Joseph Myers Dec. 11, 2014, 2:09 p.m. UTC | #3
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").

Patch
diff mbox

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;