{"thread":{"id":"20799","subject":"[PATCH] fast-import.c: Silence build warning","startedAt":"2009-08-31T11:21:34Z","lastAt":"2009-09-01T06:30:38Z","messageCount":9,"participants":["Michael Wookey","Sverre Rabbelier","Alex Riesen","Junio C Hamano","Stephen Boyd"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"122181","messageId":"d2e97e800908310421u7de8ae58o361bd64a026384bf@mail.gmail.com","threadId":"20799","inReplyTo":null,"subject":"[PATCH] fast-import.c: Silence build warning","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2009-08-31T11:21:34Z","receivedAt":"2009-08-31T11:21:34Z","isPatch":true,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not\nchecked by issuing the following notice:\n\n warning: ignoring return value of ‘strtoul’, declared with attribute\nwarn_unused_result\n\nProvide a dummy variable to keep the compiler happy.\n\nSigned-off-by: Michael Wookey \n---\n fast-import.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 7ef9865..1386e75 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,\nchar *result, int maxlen)\n {\n \tconst char *orig_src = src;\n \tchar *endp;\n+\tunsigned long int unused;\n\n \terrno = 0;\n\n-\tstrtoul(src, &endp, 10);\n+\tunused = strtoul(src, &endp, 10);\n \tif (errno || endp == src || *endp != ' ')\n \t\treturn -1;\n\n@@ -1755,7 +1756,7 @@ static int validate_raw_date(const char *src,\nchar *result, int maxlen)\n \tif (*src != '-' && *src != '+')\n \t\treturn -1;\n\n-\tstrtoul(src + 1, &endp, 10);\n+\tunused = strtoul(src + 1, &endp, 10);\n \tif (errno || endp == src || *endp || (endp - orig_src) >= maxlen)\n \t\treturn -1;\n\n-- \n1.6.4.2.236.gf324c\n"},{"id":"122184","messageId":"fabb9a1e0908310529q4c601a73t671cc2813dfdb1a3@mail.gmail.com","threadId":"20799","inReplyTo":"d2e97e800908310421u7de8ae58o361bd64a026384bf@mail.gmail.com","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-31T12:29:50Z","receivedAt":"2009-08-31T12:29:50Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Aug 31, 2009 at 04:21, Michael Wookey wrote:\n> Provide a dummy variable to keep the compiler happy.\n\nShould we not instead check the value?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"122205","messageId":"81b0412b0908311427t5b4a24ffg1d7d272669476117@mail.gmail.com","threadId":"20799","inReplyTo":"fabb9a1e0908310529q4c601a73t671cc2813dfdb1a3@mail.gmail.com","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-31T21:27:05Z","receivedAt":"2009-08-31T21:27:05Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Mon, Aug 31, 2009 at 14:29, Sverre Rabbelier wrote:\n> On Mon, Aug 31, 2009 at 04:21, Michael Wookey wrote:\n>> Provide a dummy variable to keep the compiler happy.\n>\n> Should we not instead check the value?\n\nWhy? It is endp (end of the parsed number) we're interested in.\n"},{"id":"122206","messageId":"fabb9a1e0908311442q2a56b1cft8ad7fe75bfde38c3@mail.gmail.com","threadId":"20799","inReplyTo":"81b0412b0908311427t5b4a24ffg1d7d272669476117@mail.gmail.com","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-31T21:42:14Z","receivedAt":"2009-08-31T21:42:14Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Aug 31, 2009 at 23:27, Alex Riesen wrote:\n> Why? It is endp (end of the parsed number) we're interested in.\n\nAh, my bad, I hadn't checked stroul's signature, sorry for the noise.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"122209","messageId":"d2e97e800908311631x6fdd7781v2e893d1ca62378b6@mail.gmail.com","threadId":"20799","inReplyTo":"81b0412b0908311427t5b4a24ffg1d7d272669476117@mail.gmail.com","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2009-08-31T23:31:05Z","receivedAt":"2009-08-31T23:31:05Z","isPatch":true,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"2009/9/1 Alex Riesen :\n> On Mon, Aug 31, 2009 at 14:29, Sverre Rabbelier wrote:\n>> On Mon, Aug 31, 2009 at 04:21, Michael Wookey wrote:\n>>> Provide a dummy variable to keep the compiler happy.\n>>\n>> Should we not instead check the value?\n>\n> Why? It is endp (end of the parsed number) we're interested in.\n\nGood point, perhaps the commit message should mention why we don't\nbother checking the return value. Something like this maybe?\n\n-- >8 --\ngcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not\nchecked by issuing the following notice:\n\n warning: ignoring return value of ‘strtoul’, declared with attribute\nwarn_unused_result\n\nThe return value of strtoul() isn't used because we are only interested\nin what is placed into endp. As such, provide a dummy variable to keep\nthe compiler happy.\n\nSigned-off-by: Michael Wookey \n-- >8 --\n"},{"id":"122213","messageId":"7vfxb7y2h3.fsf@alter.siamese.dyndns.org","threadId":"20799","inReplyTo":"d2e97e800908310421u7de8ae58o361bd64a026384bf@mail.gmail.com","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-31T23:42:48Z","receivedAt":"2009-08-31T23:42:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Wookey writes:\n\n> gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not\n> checked by issuing the following notice:\n>\n> warning: ignoring return value of ‘strtoul’, declared with attribute\n> warn_unused_result\n>\n> Provide a dummy variable to keep the compiler happy.\n>\n> Signed-off-by: Michael Wookey \n> ---\n> fast-import.c | 5 +++--\n> 1 files changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/fast-import.c b/fast-import.c\n> index 7ef9865..1386e75 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,\n> char *result, int maxlen)\n> {\n> \tconst char *orig_src = src;\n> \tchar *endp;\n> +\tunsigned long int unused;\n>\n> \terrno = 0;\n>\n> -\tstrtoul(src, &endp, 10);\n> +\tunused = strtoul(src, &endp, 10);\n\nIsn't this typically done by casting the expression to (void)?\n\nOtherwise a clever compiler has every right to complain \"the variable\nunused is assigned but never used.\"\n"},{"id":"122215","messageId":"d2e97e800908311655t553d6c4bo6ed45fe37819c1d8@mail.gmail.com","threadId":"20799","inReplyTo":"7vfxb7y2h3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2009-08-31T23:55:39Z","receivedAt":"2009-08-31T23:55:39Z","isPatch":true,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"2009/9/1 Junio C Hamano :\n> Michael Wookey writes:\n>\n>> gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not\n>> checked by issuing the following notice:\n>>\n>>   warning: ignoring return value of ‘strtoul’, declared with attribute\n>> warn_unused_result\n>>\n>> Provide a dummy variable to keep the compiler happy.\n>>\n>> Signed-off-by: Michael Wookey \n>> ---\n>>  fast-import.c |    5 +++--\n>>  1 files changed, 3 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/fast-import.c b/fast-import.c\n>> index 7ef9865..1386e75 100644\n>> --- a/fast-import.c\n>> +++ b/fast-import.c\n>> @@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,\n>> char *result, int maxlen)\n>>  {\n>>       const char *orig_src = src;\n>>       char *endp;\n>> +     unsigned long int unused;\n>>\n>>       errno = 0;\n>>\n>> -     strtoul(src, &endp, 10);\n>> +     unused = strtoul(src, &endp, 10);\n>\n> Isn't this typically done by casting the expression to (void)?\n\nI originally tried that - the compiler still complains.\n\n> Otherwise a clever compiler has every right to complain \"the variable\n> unused is assigned but never used.\"\n\n I get no other warnings, so does that make gcc less than clever? ;-)\n"},{"id":"122223","messageId":"4A9C9DB4.8070702@gmail.com","threadId":"20799","inReplyTo":"d2e97e800908311655t553d6c4bo6ed45fe37819c1d8@mail.gmail.com","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-09-01T04:06:12Z","receivedAt":"2009-09-01T04:06:12Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Michael Wookey wrote:\n> 2009/9/1 Junio C Hamano :\n>> Isn't this typically done by casting the expression to (void)?\n>\n> I originally tried that - the compiler still complains.\n>\n>> Otherwise a clever compiler has every right to complain \"the variable\n>> unused is assigned but never used.\n>\n> I get no other warnings, so does that make gcc less than clever? ;-) \n\nI noticed this warning recently too when I upgraded my box and a flurry\nof fwrite() unused warnings came up. Looks like ubuntu patches that\nissue[1] by arguing it's a valid programming style to\nfwrite/fflush/ferror. Perhaps this programming style could follow a\nsimilar reasoning?\n\nIt gets better though. Commit c55fae4 (fast-import.c: stricter strtoul\ncheck, silence compiler warning, 2008-12-21) made this change already.\nThen commit eb3a9dd (Remove unused function scope local variables,\n2009-03-07) came by and removed it. Unless the definition of strtoul\ndrops the attribute I fear we'll keep going back and forth.\n\n-- Footnotes --\n[1] https://lists.ubuntu.com/archives/ubuntu-devel/2009-March/027832.html\n"},{"id":"122229","messageId":"81b0412b0908312330t3135d033o14a6cb73797edf18@mail.gmail.com","threadId":"20799","inReplyTo":"d2e97e800908311655t553d6c4bo6ed45fe37819c1d8@mail.gmail.com","subject":"Re: [PATCH] fast-import.c: Silence build warning","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-09-01T06:30:38Z","receivedAt":"2009-09-01T06:30:38Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Sep 1, 2009 at 01:55, Michael Wookey wrote:\n>> Otherwise a clever compiler has every right to complain \"the variable\n>> unused is assigned but never used.\"\n>\n>  I get no other warnings, so does that make gcc less than clever? ;-)\n\nIt only does what it is instructed to do: the function is annotated with\nwarn_unused_result attribute. What really is annoying is someones\nchoice of the functions to annotate.\n"}]}