Subject: RE: bug#9064: [PATCH] coreutils-8.12 compiler warning

From: "Voelker, Bernhard" <[email-masked]>
Date: Wed, 13 Jul 2011 15:22:13 +0200

Pádraig Brady wrote:
> On 13/07/11 13:59, Voelker, Bernhard wrote:
>> Joachim Schmitz wrote:
>> 
>>> From: Voelker, Bernhard 
>>>> I remember I've seen something like (0 != (stdout_closed = true)) to make
>>>> the compiler happy.
>>>
>>> Yes, that change would make my compiler happy too.
>> 
>> A search using `find . -type f -name '*.c' | xargs grep ' if .* = '`
>> unveiled a second place in stat.c.
>> Here's a corresponding patch to avoid both warnings.
>> 
>> 
>>>>From cef2afe553f821f2eb716b90623581e790ce67a3 Mon Sep 17 00:00:00 2001
>> From: Bernhard Voelker <[email-masked]>
>> Date: Wed, 13 Jul 2011 14:50:38 +0200
>> Subject: [PATCH] maint: fix warning 'possible use of "=" where "==" was
>>  intended'
>> 
>> * src/mktemp.c: avoid warning by enclosing in double brackets
>>   and comparison "!= 0".
>> * src/stat.c: Likewise.
>> ---
>>  src/mktemp.c |    2 +-
>>  src/stat.c   |    2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/mktemp.c b/src/mktemp.c
>> index c6d0e58..c2ab025 100644
>> --- a/src/mktemp.c
>> +++ b/src/mktemp.c
>> @@ -341,7 +341,7 @@ main (int argc, char **argv)
>>        puts (dest_name);
>>        /* If we created a file, but then failed to output the file
>>           name, we should clean up the mess before failing.  */
>> -      if (!dry_run && (stdout_closed = true) && close_stream (stdout) != 0)
>> +      if (!dry_run && ((stdout_closed = true) != 0) && close_stream (stdout) != 0)
>>          {
>>            int saved_errno = errno;
>>            remove (dest_name);
>> diff --git a/src/stat.c b/src/stat.c
>> index 0ad465e..bd60457 100644
>> --- a/src/stat.c
>> +++ b/src/stat.c
>> @@ -834,7 +834,7 @@ out_mount_point (char const *filename, char *pformat, size_t prefix_len,
>>       Note we don't detect if any of the directory components
>>       are bind mounted to the same device, but that's OK
>>       since we've not directly queried them.  */
>> -  if ((mp = find_mount_point (filename, statp)))
>> +  if ((mp = find_mount_point (filename, statp)) != 0)
>>      {
>>        /* This dir might be bind mounted to another device,
>>           so we resolve the bound source in that case also.  */
>
>
> That would be slightly mangling the code to avoid a _warning_
> on old buggy compilers.

well, my intension was not to satisfy the compiler primarily,
but IMO it enhances readability: it proves that the developer
did intensionally use '=' instead of '==' ... at least in the
first case. In the second, there were already double brackets,
but I thought it'd make also that one clearer.

Also think about Joachim's words:
>>>>> Not sure whether this is bug or feature ;-)

Have a nice day,
Berny