Re: [PATCH 2/5] Check return of stat() to avoid covscan complaining

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]

On Tue, 24 Feb 2015 16:56:29 -0500 Jes Sorensen <Jes.Sorensen@redhat.com>
wrote:

> NeilBrown <neilb@suse.de> writes:
> > On Tue, 24 Feb 2015 16:00:37 -0500 Jes.Sorensen@redhat.com wrote:
> >
> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> 
> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> ---
> >>  Assemble.c | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/Assemble.c b/Assemble.c
> >> index 131f871..b392214 100644
> >> --- a/Assemble.c
> >> +++ b/Assemble.c
> >> @@ -688,7 +688,11 @@ static int load_devices(struct devs *devices, char *devmap,
> >>  			close(dfd);
> >>  		}
> >>  
> >> -		stat(devname, &stb);
> >> +		if (stat(devname, &stb)) {
> >> +			pr_err("Unsable to stat(%s) - skipping device.\n",
> >> +			       devname);
> >> +			continue;
> >> +		}
> >>  
> >>  		if (c->verbose > 0)
> >>  			pr_err("%s is identified as a member of %s, slot %d%s.\n",
> >
> > I've applied the other 4.  I think I'd rather this one was fixed by changing
> >   stat(devname,
> > to
> >   fstat(dfd,
> > and keep dfd open a bit longer.
> >
> > Does this look OK to you?
> 
> I got the warning from covscan because we ignored the return value from
> stat, so I think you still need to check the return value from fstat()
> as well.

I hope not.
You can only get errors from fstat if you do something stupid like passing
NULL as the stat pointer, or passing a non-open file descriptior.
So if covscan complains, then covscan is broken.

In contrast, stat can certainly given an error, such a ENOENT, which cannot
possibly be avoided by not being stupid.

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]