On Fri, Mar 12, 2010 at 05:27:24PM +0200, Ameya Palande wrote: > From: Ameya Palande <ameya.palande at nokia.com> > This patch fixes following warning: > > kexec/firmware_memmap.c: In function ?parse_numeric_sysfs?: > kexec/firmware_memmap.c:70: error: ignoring return value of ?fgets?, declared > with attribute warn_unused_result > kexec/firmware_memmap.c: In function ?parse_string_sysfs?: > kexec/firmware_memmap.c:103: error: ignoring return value of ?fgets?, declared > with attribute warn_unused_result > > Signed-off-by: Ameya Palande <ameya.palande at nokia.com> > --- > kexec/firmware_memmap.c | 32 +++++++++++++++++++------------- > 1 files changed, 19 insertions(+), 13 deletions(-) > > diff --git a/kexec/firmware_memmap.c b/kexec/firmware_memmap.c > index 5a101fb..70e0f74 100644 > --- a/kexec/firmware_memmap.c > +++ b/kexec/firmware_memmap.c > @@ -67,11 +67,14 @@ static unsigned long long parse_numeric_sysfs(const char *filename) > return ULLONG_MAX; > } > > - fgets(linebuffer, BUFSIZ, fp); > - linebuffer[BUFSIZ-1] = 0; > + if(fgets(linebuffer, BUFSIZ, fp)) { > + linebuffer[BUFSIZ-1] = 0; > > - /* let strtoll() detect the base */ > - retval = strtoll(linebuffer, NULL, 0); > + /* let strtoll() detect the base */ > + retval = strtoll(linebuffer, NULL, 0); > + } else { > + retval = ULLONG_MAX; > + } > > fclose(fp); Could we change the style of this to something less-nested? Also, there needs to be a space after if. unsigned long long retval = ULLONG_MAX; ... if (!fgets(linebuffer, BUFSIZ, fp)) goto err; linebuffer[BUFSIZ-1] = 0; /* let strtoll() detect the base */ retval = strtoll(linebuffer, NULL, 0); err: fclose(fp); > > @@ -100,18 +103,21 @@ static char *parse_string_sysfs(const char *filename) > return NULL; > } > > - fgets(linebuffer, BUFSIZ, fp); > - linebuffer[BUFSIZ-1] = 0; > - > - /* truncate trailing newline(s) */ > - end = linebuffer + strlen(linebuffer) - 1; > - while (*end == '\n') > - *end-- = 0; > + if (fgets(linebuffer, BUFSIZ, fp)) { > + linebuffer[BUFSIZ-1] = 0; > > - fclose(fp); > + /* truncate trailing newline(s) */ > + end = linebuffer + strlen(linebuffer) - 1; > > - return linebuffer; > + while (*end == '\n') > + *end-- = 0; > > + fclose(fp); > + return linebuffer; > + } else { > + fclose(fp); > + return NULL; > + } > } Again, I would prefer something less-nested.