GNU bug report logs - #17667
[PATCH] df: Initialize a variable to squash a compiler warning

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17667 in the body.
You can then email your comments to 17667 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ben Walton <bdwalton <at> gmail.com>
To: bug-coreutils <at> gnu.org
Cc: Ben Walton <bdwalton <at> gmail.com>
Subject: [PATCH] df: Initialize a variable to squash a compiler warning
Date: Mon,  2 Jun 2014 09:09:30 +0100

* src/df.c: get_dev - With strict error checking, gcc complained that
            v may have been used prior to initialization. To avoid
            this, initialize to NULL.

Signed-off-by: Ben Walton <bdwalton <at> gmail.com>
---
 src/df.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/df.c b/src/df.c
index 01ecca6..059c958 100644
--- a/src/df.c
+++ b/src/df.c
@@ -924,7 +924,7 @@ get_dev (char const *disk, char const *mount_point, char const* file,
       char buf[LONGEST_HUMAN_READABLE + 2];
       char *cell;
 
-      struct field_values_t *v;
+      struct field_values_t *v = NULL;
       switch (columns[col]->field_type)
         {
         case BLOCK_FLD:
@@ -934,7 +934,7 @@ get_dev (char const *disk, char const *mount_point, char const* file,
           v = &inode_values;
           break;
         case OTHER_FLD:
-          v = NULL;
+          /* Rely on NULL initialization. */
           break;
         default:
           assert (!"bad field_type");
-- 
1.9.1

Message #10 received at 17667-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Ben Walton <bdwalton <at> gmail.com>
Cc: 17667-done <at> debbugs.gnu.org
Subject: Re: bug#17667: [PATCH] df: Initialize a variable to squash a compiler
 warning
Date: Mon, 02 Jun 2014 20:22:19 +0100

On 06/02/2014 09:09 AM, Ben Walton wrote:
> * src/df.c: get_dev - With strict error checking, gcc complained that
>             v may have been used prior to initialization. To avoid
>             this, initialize to NULL.
> 
> Signed-off-by: Ben Walton <bdwalton <at> gmail.com>
> ---
>  src/df.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/df.c b/src/df.c
> index 01ecca6..059c958 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -924,7 +924,7 @@ get_dev (char const *disk, char const *mount_point, char const* file,
>        char buf[LONGEST_HUMAN_READABLE + 2];
>        char *cell;
>  
> -      struct field_values_t *v;
> +      struct field_values_t *v = NULL;
>        switch (columns[col]->field_type)
>          {
>          case BLOCK_FLD:
> @@ -934,7 +934,7 @@ get_dev (char const *disk, char const *mount_point, char const* file,
>            v = &inode_values;
>            break;
>          case OTHER_FLD:
> -          v = NULL;
> +          /* Rely on NULL initialization. */
>            break;
>          default:
>            assert (!"bad field_type");

This is because assert() is not declared __noreturn__ on Solaris 10.
That can be an important admonition for a compiler
so I'm wondering should be detect this and provide a __noreturn__ wrapper.

Anyway what I don't want to do is change the current logic
to avoid such bogus warnings. What we could do here
is to tweak the assert path only to avoid the warning as follows.
OK to push the following instead in your name?

thanks,
Pádraig.

diff --git a/src/df.c b/src/df.c
index 82b0c5f..c08ad97 100644
--- a/src/df.c
+++ b/src/df.c
@@ -953,6 +953,7 @@ get_dev (char const *disk, char const *mount_point, char con
           v = NULL;
           break;
         default:
+          v = NULL; /* avoid warnings where assert() is not __noreturn__.  */
           assert (!"bad field_type");
         }

Message #13 received at 17667-done <at> debbugs.gnu.org (full text, mbox):

From: Ben Walton <bdwalton <at> gmail.com>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 17667-done <at> debbugs.gnu.org
Subject: Re: bug#17667: [PATCH] df: Initialize a variable to squash a compiler
 warning
Date: Mon, 2 Jun 2014 20:29:27 +0100

On Mon, Jun 2, 2014 at 8:22 PM, Pádraig Brady <P <at> draigbrady.com> wrote:
> On 06/02/2014 09:09 AM, Ben Walton wrote:
>> * src/df.c: get_dev - With strict error checking, gcc complained that
>>             v may have been used prior to initialization. To avoid
>>             this, initialize to NULL.
>>
>> Signed-off-by: Ben Walton <bdwalton <at> gmail.com>
>> ---
>>  src/df.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/df.c b/src/df.c
>> index 01ecca6..059c958 100644
>> --- a/src/df.c
>> +++ b/src/df.c
>> @@ -924,7 +924,7 @@ get_dev (char const *disk, char const *mount_point, char const* file,
>>        char buf[LONGEST_HUMAN_READABLE + 2];
>>        char *cell;
>>
>> -      struct field_values_t *v;
>> +      struct field_values_t *v = NULL;
>>        switch (columns[col]->field_type)
>>          {
>>          case BLOCK_FLD:
>> @@ -934,7 +934,7 @@ get_dev (char const *disk, char const *mount_point, char const* file,
>>            v = &inode_values;
>>            break;
>>          case OTHER_FLD:
>> -          v = NULL;
>> +          /* Rely on NULL initialization. */
>>            break;
>>          default:
>>            assert (!"bad field_type");
>
> This is because assert() is not declared __noreturn__ on Solaris 10.
> That can be an important admonition for a compiler
> so I'm wondering should be detect this and provide a __noreturn__ wrapper.

Ah. Ok. That makes sense. I think providing this wrapper is likely a
good thing although this is a corner case we're catching here.

>
> Anyway what I don't want to do is change the current logic
> to avoid such bogus warnings. What we could do here
> is to tweak the assert path only to avoid the warning as follows.
> OK to push the following instead in your name?

Ack, that's fine with me.

Thanks
-Ben

>
> thanks,
> Pádraig.
>
> diff --git a/src/df.c b/src/df.c
> index 82b0c5f..c08ad97 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -953,6 +953,7 @@ get_dev (char const *disk, char const *mount_point, char con
>            v = NULL;
>            break;
>          default:
> +          v = NULL; /* avoid warnings where assert() is not __noreturn__.  */
>            assert (!"bad field_type");
>          }
>
>



-- 
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------

Message #16 received at 17667 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: 17667 <at> debbugs.gnu.org, P <at> draigBrady.com, bdwalton <at> gmail.com
Subject: Re: bug#17667: [PATCH] df: Initialize a variable to squash a compiler
 warning
Date: Mon, 02 Jun 2014 13:55:06 -0600

[Message part 1 (text/plain, inline)]

On 06/02/2014 01:22 PM, Pádraig Brady wrote:

>>          default:
>>            assert (!"bad field_type");
> 
> This is because assert() is not declared __noreturn__ on Solaris 10.

Huh? assert(nonzero) returns normally.  I thought __noreturn__ was
reserved for functions like exit() that really cannot return; assert(0)
does not return, but that does not mean it has the same semantics as
exit().  (Well, technically assert() is a macro, while the __noreturn__
semantics have to be attached to a function invoked by the macro - but
how do you portably determine which function is going to be invoked by
the macro?)

> That can be an important admonition for a compiler
> so I'm wondering should be detect this and provide a __noreturn__ wrapper.

In glibc, it is not assert() that is marked __noreturn__, but the
__assert_fail() internal function call that only gets triggered for
assert(0).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.