Re: [libvirt] [PATCH] usbFindBusByVendor: don't leak a DIR buffer and FD

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.