Re: [Qemu-devel] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets

[Eric Blake]

On 08/13/2012 07:44 AM, Corey Bryant wrote:
> I'll send a new version shortly with these updates also.
> 

>>> +
>>> +        ret = monitor_fdset_dup_fd_add(fdset_id, dupfd);
>>> +        if (ret == -1) {
>>> +            close(dupfd);
>>> +            return -1;
>>
>> This function appears to promise a reasonable errno on failure.

Actually, looking at that function again,


+int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+{
+    MonFdset *mon_fdset;
+    MonFdsetFd *mon_fdset_fd_dup;
+
+    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+        if (mon_fdset->id != fdset_id) {
+            continue;
+        }
+        QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {
+            if (mon_fdset_fd_dup->fd == dup_fd) {
+                return -1;
+            }
+        }
+        mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));
+        mon_fdset_fd_dup->fd = dup_fd;
+        QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next);
+        return 0;
+    }
+    return -1;
+}

The only way it could fail is if we are trying to add an fd that is
already in the set, or if we don't find mon_fdset; both of which would
indicate logic bugs earlier in our program.  Would it be worth asserting
that these conditions are impossible, and making this function return
void (the addition is always successful if it returns, since g_malloc0
aborts rather than failing with ENOMEM)?

And the more I think about it, the more I think that qemu_open MUST
provide a sane errno value on exit, so you need to make sure that all
exit paths out of qemu_open have a sensible errno (whether or not the
helper functions also have to leave errno sane is a matter of taste).

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature