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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list