Re: [Qemu-devel] [PULL 00/11] 9pfs patches for 2.10 20170525

[Daniel P. Berrange]

On Tue, May 30, 2017 at 12:21:12PM +0200, Greg Kurz wrote:
> On Tue, 30 May 2017 10:42:12 +0100
> Stefan Hajnoczi <address@hidden> wrote:
> 
> > On Mon, May 29, 2017 at 11:05:29AM +0200, Greg Kurz wrote:
> > > The following changes since commit 
> > > 9964e96dc9999cf7f7c936ee854a795415d19b60:
> > > 
> > >   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into 
> > > staging (2017-05-23 15:01:31 +0100)
> > > 
> > > are available in the git repository at:
> > > 
> > >   https://github.com/gkurz/qemu.git tags/for-upstream
> > > 
> > > for you to fetch changes up to f0a4da86cff2e600255324793daddd7ce59b9862:
> > > 
> > >   9pfs: local: metadata file for the VirtFS root (2017-05-25 10:30:14 
> > > +0200)
> > > 
> > > ----------------------------------------------------------------
> > > Various bugfixes and code cleanups. Most notably, it fixes metadata 
> > > handling in
> > > mapped-file security mode (especially for the virtfs root).  
> > 
> > Please fix the compiler warning reported by patchew.
> > 
> 
> In file included from 
> /var/tmp/patchew-tester-tmp-3cnydauu/src/hw/9pfs/9p-local.c:18:0:
> /var/tmp/patchew-tester-tmp-3cnydauu/src/hw/9pfs/9p-local.c: In function 
> ‘local_set_mapped_file_attrat’:
> /var/tmp/patchew-tester-tmp-3cnydauu/src/hw/9pfs/9p-util.h:19:5: error: 
> ‘map_dirfd’ may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>      close(fd);
>      ^~~~~~~~~
> /var/tmp/patchew-tester-tmp-3cnydauu/src/hw/9pfs/9p-local.c:235:9: note: 
> ‘map_dirfd’ was declared here
>      int map_dirfd, map_fd;
>          ^~~~~~~~~
> cc1: all warnings being treated as errors
> 
> This is a false positive: map_dirfd is necessarily initialized, but I guess
> gcc isn't smart enough to see that :-\
> 
> It is acceptable to close(-1) so I guess I'll just do:
> 
> -    int map_dirfd, map_fd;
> +    int map_dirfd = -1, map_fd;

By 'acceptable' I guess you mean it'll return EBADF. I would not
be surprised, however, if coverity were to then complain if it sees
code path where we pass -1 to close, since it is indicative of a
potential bug. So in addition to your initialization, also protecting
the close() call with '!= -1' condition is a safer approach.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|