On Thu, Jun 02, 2011 at 01:16:59PM -0500, Jonathan Nieder wrote:
>severity 625357 normal
>retitle 625357 gcc -Wunused-but-set-variable should not be implied by -Wall (?)
>tags 625357 = upstream moreinfo
>quit
>
>Hi again,
>
>Steve McIntyre wrote:
>
>> I'll remove the -Werror to stop gcc breaking the build here, but I
>> definitely believe that gcc is doing the wrong thing here.
>> Technically, yes - the variables are set but unused. However, this is
>> a far higher level of pedantry than is warranted for -Wall. The code
>> being complained about is perfectly valid, typical of the defensive
>> programming pattern of "initialise all variables".
>
>To clarify: here's a demo of the behavior of gcc-4.4.
>
> $ program_one='int main(void) { int x = 5; return 0; }'
> $ program_two='int main(void) { int x; x = 5; return 0; }'
> $ echo "$program_one" | gcc-4.4 -Wall -x c -
> <stdin>: In function ‘main’:
> <stdin>:1: warning: unused variable ‘x’
> $ echo "$program_two" | gcc-4.4 -Wall -x c -
Sure, that's confused. :-)
>So imho gcc before v4.6 is just fundamentally confused. Which means
>there are a number of ways one could go with this. Do you mean:
>
> A. Unused variables are not a big deal, and they belong in -Wextra,
> not -Wall.
No.
> B. New warnings are a pain in the neck and should go in -Wextra
> during a transition period.
Maybe, but a lot of people will never see them anyway until they cause
build failures. :-)
> C. The idiom
>
> ssize_t unused;
> /*
> * Yes, there might be an error, dear gcc -D_FORTIFY_SOURCE,
> * but we want to ignore it.
> */
> unused = write(...);
>
> in place of, say,
>
> /* loop on partial writes and EINTR */
> xwrite(...);
>
> is good style and the -Wunused-but-set-variable warning is
> fundamentally misguided.
I thought I was looking at a different case, but I've just re-tested
various things to respond to you here. It looks like I've missed
*exactly* the thing gcc was complaining about, and it is case C
here. Now I can see that, I'm less unhappy about the warning.
My code is analogous to your example code:
static int parse_md5_entry(char *md5_entry)
{
int error = 0;
char *file_name = NULL;
char *md5 = NULL;
unsigned char bin_md5[16];
int i;
...
/* code that does the real meat of the work to fill in "md5" and
"filename" , but doesn't set/use "error" at all */
...
error = add_md5_entry(UNKNOWN, md5, file_name);
return 0;
}
To explain a bit more: the warning reported by gcc (unhelpfully)
points at the declaration/initialisation of "error", which led me to
(incorrectly) think that it was complaining about set-but-unused for
the initialisation. With minor tweaks, I can see that it's complaining
about the return value from add_md5_entry() being ignored but doesn't
*say* that. :-(
I'm about to update the code around that area now. More accurate
diagnostics from gcc would be a major improvement, I think!
--
Steve McIntyre, Cambridge, UK. st...@einval.com
"...In the UNIX world, people tend to interpret `non-technical user'
as meaning someone who's only ever written one device driver." -- Daniel Pead
--
To UNSUBSCRIBE, email to debian-gcc-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110603153043.ge28...@einval.com
