[libvirt] [PATCH] don't leak a file descriptor on failed pciGetDevice call

[Daniel P. Berrange]

On Tue, Mar 03, 2009 at 11:21:16AM +0100, Jim Meyering wrote:
> Daniel P. Berrange wrote:
> > On Tue, Mar 03, 2009 at 11:13:40AM +0100, Jim Meyering wrote:
> >> This loop would mistakenly return early (skipping the closedir)
> >> upon pciGetDevice failure.
> >>
> >> >From 2d4d1d25edf8f1c3f4770707215bba67d73fd59f Mon Sep 17 00:00:00 2001
> >> From: Jim Meyering <meyering at redhat.com>
> >> Date: Tue, 3 Mar 2009 11:11:07 +0100
> >> Subject: [PATCH] don't leak a file descriptor on failed pciGetDevice call
> >
> > ACK, but...
> >
> >> @@ -823,7 +827,7 @@ void
> >>  pciFreeDevice(virConnectPtr conn ATTRIBUTE_UNUSED, pciDevice *dev)
> >>  {
> >>      VIR_DEBUG("%s %s: freeing", dev->id, dev->name);
> >> -    if (dev->fd)
> >> +    if (dev->fd >= 0)
> >>          close(dev->fd);
> >>      VIR_FREE(dev);
> >>  }
> >
> > With this change you also need to fix the bug in pciGetDevice() which
> > leaves 'fd' initialized to 0 rather than setting it to -1
> 
> Ha.  Good catch.
> That change did not belong in this patch (note lack of mention in log message).
> It belongs in a separate patch.

Yes it does because, previously it would be harmless due to

  if (dev->fd)

only check for fd > 0, but with this new patch applied it turns into a
real bug, since the check is now fd >= 0. 

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|