On Tue, Jan 26, 2010 at 03:59:08PM +0100, Jim Meyering wrote:
Eric Blake wrote:
According to Jim Meyering on 1/25/2010 8:55 AM:
Subject: [PATCH] hostusb: closedir only if non-NULL; rename labels: s/error/cleanup/
* src/util/hostusb.c (usbSysReadFile): Rename labels s/error/cleanup/ (usbFindBusByVendor): Likewise. And closedir only if non-NULL. @@ -150,8 +150,9 @@ static int usbFindBusByVendor(virConnectPtr conn, else ret = 0;
-error: - closedir (dir); +cleanup: + if (dir) + closedir (dir);
Should errno be saved and restored around this point, to avoid it being arbitrarily changed by closedir?
Thanks for looking. Technically you're right, and I'll merge this:
diff --git a/src/util/hostusb.c b/src/util/hostusb.c index 72d0833..71f6435 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -151,8 +151,11 @@ static int usbFindBusByVendor(virConnectPtr conn, ret = 0;
cleanup: - if (dir) + if (dir) { + int saved_errno = errno; closedir (dir); + errno = saved_errno; + } return ret; }
However, no caller that I have seen uses errno (I looked at all direct callers and stopped after getting to 4th or 5th-level indirect callers in one part of the call tree), so currently it makes no difference.
Yep, generally speaking if a caller needs to be given back the actual errno value, then the function should be returing '-errno' instead of the fixed -1. 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 :|