threads / patch / 20799

patchfast-import.c: Silence build warning

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

## tl;dr

9 messages between Aug 31, 2009 and Sep 1, 2009. Diffs are folded; open one to read it.

replies: 8people: 5as markdown or json

Michael Wookey· Aug 31, 2009, 11:21 UTC · lore

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(-)
Show changes to fast-import.c +3 −2
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
Sverre Rabbelier· Aug 31, 2009, 12:29 UTC · re: Michael Wookey · lore

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

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
Alex Riesen· Aug 31, 2009, 21:27 UTC · re: Sverre Rabbelier · lore

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

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.
Sverre Rabbelier· Aug 31, 2009, 21:42 UTC · re: Alex Riesen · lore

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

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
Michael Wookey· Aug 31, 2009, 23:31 UTC · re: Alex Riesen · lore

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

2009/9/1 Alex Riesen <raa.lkml@gmail.com>:
Show 7 quoted lines
> 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 --
Junio C Hamano· Aug 31, 2009, 23:42 UTC · re: Michael Wookey · lore

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

Michael Wookey <michaelwookey@gmail.com> writes:
Show 28 quoted lines
> 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."

Michael Wookey· Aug 31, 2009, 23:55 UTC · re: Junio C Hamano · lore

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

2009/9/1 Junio C Hamano <gitster@pobox.com>:
Show 32 quoted lines
> 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? ;-)
Stephen Boyd· Sep 1, 2009, 04:06 UTC · re: Michael Wookey · lore

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

Michael Wookey wrote:
Show 9 quoted lines
> 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

Alex Riesen· Sep 1, 2009, 06:30 UTC · re: Michael Wookey · lore

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

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.

← back to recent threads