[PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().

[PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().

[Riku Voipio]

In commit:

d37d63d symbols: prefix static symbols with their source file names

An unchecked fgets was added. This causes a compile error:

symbols.c: In function 'read_symbol':
symbols.c:181:3: error: ignoring return value of 'fgets', declared with
attribute warn_unused_result [-Werror=unused-result]
   fgets(str, 500, in); /* discard rest of line */
   ^

Paper over the warning like in the other similar fgets-on-error-path
earlier in the same file.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
 xen/tools/symbols.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/tools/symbols.c b/xen/tools/symbols.c
index dbf6a1a..7e75be2 100644
--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -178,8 +178,8 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 
  skip_tail:
 	if (input_format == fmt_sysv)
-		fgets(str, 500, in); /* discard rest of line */
-
+		if (fgets(str, 500, in) == NULL) /* discard rest of line */
+			return -1; /* must check fgets result */
 	return rc;
 }
 
-- 
2.6.2

Re: [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().

[Jan Beulich]

>>> On 04.11.15 at 12:39, <riku.voipio@linaro.org> wrote:
> In commit:
> 
> d37d63d symbols: prefix static symbols with their source file names
> 
> An unchecked fgets was added. This causes a compile error:
> 
> symbols.c: In function 'read_symbol':
> symbols.c:181:3: error: ignoring return value of 'fgets', declared with
> attribute warn_unused_result [-Werror=unused-result]
>    fgets(str, 500, in); /* discard rest of line */
>    ^
> 
> Paper over the warning like in the other similar fgets-on-error-path
> earlier in the same file.

But the two cases are dissimilar - the original one skips a line the
format of which is not recognized, while here you may be converting
success into an error. (I did notice the comment on the earlier fgets(),
but since so far I didn't get any compiler warning on any system I
built this on, I assumed we'd be fine without the check, since if we
need the check, then it will end up even more clumsy than the other
one.)

> --- a/xen/tools/symbols.c
> +++ b/xen/tools/symbols.c
> @@ -178,8 +178,8 @@ static int read_symbol(FILE *in, struct sym_entry *s)
>  
>   skip_tail:
>  	if (input_format == fmt_sysv)
> -		fgets(str, 500, in); /* discard rest of line */
> -
> +		if (fgets(str, 500, in) == NULL) /* discard rest of line */
> +			return -1; /* must check fgets result */

As to formal things - two such consecutive if()-s should be combined.
Since we really want to ignore the return value here, perhaps just
cast the function result to void? (I admit that I don't recall whether
this would take care of that compiler warning.)

Jan

Re: [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().

[Haozhong Zhang]

On 11/04/15 07:04, Jan Beulich wrote:
> >>> On 04.11.15 at 12:39, <riku.voipio@linaro.org> wrote:
> > In commit:
> > 
> > d37d63d symbols: prefix static symbols with their source file names
> > 
> > An unchecked fgets was added. This causes a compile error:
> > 
> > symbols.c: In function 'read_symbol':
> > symbols.c:181:3: error: ignoring return value of 'fgets', declared with
> > attribute warn_unused_result [-Werror=unused-result]
> >    fgets(str, 500, in); /* discard rest of line */
> >    ^
> > 
> > Paper over the warning like in the other similar fgets-on-error-path
> > earlier in the same file.
> 
> But the two cases are dissimilar - the original one skips a line the
> format of which is not recognized, while here you may be converting
> success into an error. (I did notice the comment on the earlier fgets(),
> but since so far I didn't get any compiler warning on any system I
> built this on, I assumed we'd be fine without the check, since if we
> need the check, then it will end up even more clumsy than the other
> one.)
>

Hi Riku and Jan,

Will there be any fix for this error? I got the same error when
compiling Xen (commit 6f04de6) by gcc 4.8.4 on Ubuntu 14.04.3. And
adding "(void)" ahead of fgets() in the existing code cannot eliminate
the error/warning message.

Thanks,
Haozhong

> > --- a/xen/tools/symbols.c
> > +++ b/xen/tools/symbols.c
> > @@ -178,8 +178,8 @@ static int read_symbol(FILE *in, struct sym_entry *s)
> >  
> >   skip_tail:
> >  	if (input_format == fmt_sysv)
> > -		fgets(str, 500, in); /* discard rest of line */
> > -
> > +		if (fgets(str, 500, in) == NULL) /* discard rest of line */
> > +			return -1; /* must check fgets result */
> 
> As to formal things - two such consecutive if()-s should be combined.
> Since we really want to ignore the return value here, perhaps just
> cast the function result to void? (I admit that I don't recall whether
> this would take care of that compiler warning.)
> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().

[Jan Beulich]

>>> On 09.11.15 at 06:03, <haozhong.zhang@intel.com> wrote:
> On 11/04/15 07:04, Jan Beulich wrote:
>> >>> On 04.11.15 at 12:39, <riku.voipio@linaro.org> wrote:
>> > In commit:
>> > 
>> > d37d63d symbols: prefix static symbols with their source file names
>> > 
>> > An unchecked fgets was added. This causes a compile error:
>> > 
>> > symbols.c: In function 'read_symbol':
>> > symbols.c:181:3: error: ignoring return value of 'fgets', declared with
>> > attribute warn_unused_result [-Werror=unused-result]
>> >    fgets(str, 500, in); /* discard rest of line */
>> >    ^
>> > 
>> > Paper over the warning like in the other similar fgets-on-error-path
>> > earlier in the same file.
>> 
>> But the two cases are dissimilar - the original one skips a line the
>> format of which is not recognized, while here you may be converting
>> success into an error. (I did notice the comment on the earlier fgets(),
>> but since so far I didn't get any compiler warning on any system I
>> built this on, I assumed we'd be fine without the check, since if we
>> need the check, then it will end up even more clumsy than the other
>> one.)
>>
> 
> Hi Riku and Jan,
> 
> Will there be any fix for this error? I got the same error when
> compiling Xen (commit 6f04de6) by gcc 4.8.4 on Ubuntu 14.04.3. And
> adding "(void)" ahead of fgets() in the existing code cannot eliminate
> the error/warning message.

I expect / welcome an (updated) patch.

Jan

Re: [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().

[Riku Voipio]

Hi,

On 9 November 2015 at 10:10, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 09.11.15 at 06:03, <haozhong.zhang@intel.com> wrote:
>> On 11/04/15 07:04, Jan Beulich wrote:
>>> >>> On 04.11.15 at 12:39, <riku.voipio@linaro.org> wrote:
>>> > In commit:
>>> >
>>> > d37d63d symbols: prefix static symbols with their source file names
>>> >
>>> > An unchecked fgets was added. This causes a compile error:
>>> >
>>> > symbols.c: In function 'read_symbol':
>>> > symbols.c:181:3: error: ignoring return value of 'fgets', declared with
>>> > attribute warn_unused_result [-Werror=unused-result]
>>> >    fgets(str, 500, in); /* discard rest of line */
>>> >    ^
>>> >
>>> > Paper over the warning like in the other similar fgets-on-error-path
>>> > earlier in the same file.
>>>
>>> But the two cases are dissimilar - the original one skips a line the
>>> format of which is not recognized, while here you may be converting
>>> success into an error. (I did notice the comment on the earlier fgets(),
>>> but since so far I didn't get any compiler warning on any system I
>>> built this on, I assumed we'd be fine without the check, since if we
>>> need the check, then it will end up even more clumsy than the other
>>> one.)
>>>
>>
>> Hi Riku and Jan,
>>
>> Will there be any fix for this error? I got the same error when
>> compiling Xen (commit 6f04de6) by gcc 4.8.4 on Ubuntu 14.04.3. And
>> adding "(void)" ahead of fgets() in the existing code cannot eliminate
>> the error/warning message.

> I expect / welcome an (updated) patch.

Sorry, I missed this mail thread. I'll dive into fixing the patch asap.

Riku

Re: [PATCH] symbols.c: Avoid warn_unused_result build failure on fgets().

[Ian Campbell]

On Wed, 2015-11-04 at 13:39 +0200, Riku Voipio wrote:

Not to do with this patch (and this suggestion wouldn't have helped in this
case) but IIRC Linaro tests the xen.git#staging branch.

We recently introduced a new "smoke test" phase which is a quick[0] test
which runs on staging before the regular big test runs to check for obvious
errors, build problems, simple guest booting etc.

The sequence of tests on xen's development branch is now:
 * A committer pushes to xen.git#staging
 * osstest runs smoke tests on staging and propagates passes to
   xen.git#smoked
 * osstest runs full suite of tests tests on smoked and propagates passes
   to xen.git#master

I mention this because perhaps Linaro would want to switch to running their
tests on #smoked instead of #staging, and avoid the cases where things are
completely knackered?

In this case the compiler which osstest uses apparently doesn't warn about
fgets() not being checked, so it wouldn't actually have helped...

Ian.

[0] ~2 hours turnaround, compared with a day or more for regular test
flights.