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. Even if a caller did use errno, it'd be unusable some of the time because this function calls usbSysReadFile, which also clobbers errno via its free-upon-cleanup calls. BTW, I noticed that this function treats a failed readdir just like end-of-directory (i.e., ignores it), but I didn't bother to fix that. There are far bigger fish to fry, currently. But we'll have to start someday.