[PATCH] fast-import.c: Silence build warning

[PATCH] fast-import.c: Silence build warning

[Michael Wookey]

gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
checked by issuing the following notice:

  warning: ignoring return value of ‘strtoul’, declared with attribute
warn_unused_result

Provide a dummy variable to keep the compiler happy.

Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
---
 fast-import.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 7ef9865..1386e75 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,
char *result, int maxlen)
 {
 	const char *orig_src = src;
 	char *endp;
+	unsigned long int unused;

 	errno = 0;

-	strtoul(src, &endp, 10);
+	unused = strtoul(src, &endp, 10);
 	if (errno || endp == src || *endp != ' ')
 		return -1;

@@ -1755,7 +1756,7 @@ static int validate_raw_date(const char *src,
char *result, int maxlen)
 	if (*src != '-' && *src != '+')
 		return -1;

-	strtoul(src + 1, &endp, 10);
+	unused = strtoul(src + 1, &endp, 10);
 	if (errno || endp == src || *endp || (endp - orig_src) >= maxlen)
 		return -1;

-- 
1.6.4.2.236.gf324c

Re: [PATCH] fast-import.c: Silence build warning

[Sverre Rabbelier]

Heya,

On Mon, Aug 31, 2009 at 04:21, Michael Wookey<michaelwookey@gmail.com> wrote:
> Provide a dummy variable to keep the compiler happy.

Should we not instead check the value?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] fast-import.c: Silence build warning

[Alex Riesen]

On Mon, Aug 31, 2009 at 14:29, Sverre Rabbelier<srabbelier@gmail.com> wrote:
> On Mon, Aug 31, 2009 at 04:21, Michael Wookey<michaelwookey@gmail.com> wrote:
>> Provide a dummy variable to keep the compiler happy.
>
> Should we not instead check the value?

Why? It is endp (end of the parsed number) we're interested in.

Re: [PATCH] fast-import.c: Silence build warning

[Sverre Rabbelier]

Heya,

On Mon, Aug 31, 2009 at 23:27, Alex Riesen<raa.lkml@gmail.com> wrote:
> Why? It is endp (end of the parsed number) we're interested in.

Ah, my bad, I hadn't checked stroul's signature, sorry for the noise.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] fast-import.c: Silence build warning

[Michael Wookey]

2009/9/1 Alex Riesen <raa.lkml@gmail.com>:
> On Mon, Aug 31, 2009 at 14:29, Sverre Rabbelier<srabbelier@gmail.com> wrote:
>> On Mon, Aug 31, 2009 at 04:21, Michael Wookey<michaelwookey@gmail.com> wrote:
>>> Provide a dummy variable to keep the compiler happy.
>>
>> Should we not instead check the value?
>
> Why? It is endp (end of the parsed number) we're interested in.

Good point, perhaps the commit message should mention why we don't
bother checking the return value. Something like this maybe?

-- >8 --
gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
checked by issuing the following notice:

 warning: ignoring return value of ‘strtoul’, declared with attribute
warn_unused_result

The return value of strtoul() isn't used because we are only interested
in what is placed into endp.  As such, provide a dummy variable to keep
the compiler happy.

Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
-- >8 --

Re: [PATCH] fast-import.c: Silence build warning

[Junio C Hamano]

Michael Wookey <michaelwookey@gmail.com> writes:

> gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
> checked by issuing the following notice:
>
>   warning: ignoring return value of ‘strtoul’, declared with attribute
> warn_unused_result
>
> Provide a dummy variable to keep the compiler happy.
>
> Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
> ---
>  fast-import.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index 7ef9865..1386e75 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,
> char *result, int maxlen)
>  {
>  	const char *orig_src = src;
>  	char *endp;
> +	unsigned long int unused;
>
>  	errno = 0;
>
> -	strtoul(src, &endp, 10);
> +	unused = strtoul(src, &endp, 10);

Isn't this typically done by casting the expression to (void)?

Otherwise a clever compiler has every right to complain "the variable
unused is assigned but never used."

Re: [PATCH] fast-import.c: Silence build warning

[Michael Wookey]

2009/9/1 Junio C Hamano <gitster@pobox.com>:
> Michael Wookey <michaelwookey@gmail.com> writes:
>
>> gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
>> checked by issuing the following notice:
>>
>>   warning: ignoring return value of ‘strtoul’, declared with attribute
>> warn_unused_result
>>
>> Provide a dummy variable to keep the compiler happy.
>>
>> Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
>> ---
>>  fast-import.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fast-import.c b/fast-import.c
>> index 7ef9865..1386e75 100644
>> --- a/fast-import.c
>> +++ b/fast-import.c
>> @@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,
>> char *result, int maxlen)
>>  {
>>       const char *orig_src = src;
>>       char *endp;
>> +     unsigned long int unused;
>>
>>       errno = 0;
>>
>> -     strtoul(src, &endp, 10);
>> +     unused = strtoul(src, &endp, 10);
>
> Isn't this typically done by casting the expression to (void)?

I originally tried that - the compiler still complains.

> Otherwise a clever compiler has every right to complain "the variable
> unused is assigned but never used."

 I get no other warnings, so does that make gcc less than clever? ;-)

Re: [PATCH] fast-import.c: Silence build warning

[Stephen Boyd]

Michael Wookey wrote:
> 2009/9/1 Junio C Hamano <gitster@pobox.com>:
>> Isn't this typically done by casting the expression to (void)?
>
> I originally tried that - the compiler still complains.
>
>> Otherwise a clever compiler has every right to complain "the variable
>> unused is assigned but never used.
>
> I get no other warnings, so does that make gcc less than clever?  ;-) 

I noticed this warning recently too when I upgraded my box and a flurry
of fwrite() unused warnings came up. Looks like ubuntu patches that
issue[1] by arguing it's a valid programming style to
fwrite/fflush/ferror. Perhaps this programming style could follow a
similar reasoning?

It gets better though. Commit c55fae4 (fast-import.c: stricter strtoul
check, silence compiler warning, 2008-12-21) made this change already.
Then commit eb3a9dd (Remove unused function scope local variables,
2009-03-07) came by and removed it. Unless the definition of strtoul
drops the attribute I fear we'll keep going back and forth.

-- Footnotes --
[1] https://lists.ubuntu.com/archives/ubuntu-devel/2009-March/027832.html

Re: [PATCH] fast-import.c: Silence build warning

[Alex Riesen]

On Tue, Sep 1, 2009 at 01:55, Michael Wookey<michaelwookey@gmail.com> wrote:
>> Otherwise a clever compiler has every right to complain "the variable
>> unused is assigned but never used."
>
>  I get no other warnings, so does that make gcc less than clever? ;-)

It only does what it is instructed to do: the function is annotated with
warn_unused_result attribute. What really is annoying is someones
choice of the functions to annotate.