Re: [PATCH v5 1/8] monitor: simplify functions for getting a dup'd fdset entry

[Daniel P . Berrangé]

On Thu, Sep 03, 2020 at 10:52:40AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > Currently code has to call monitor_fdset_get_fd, then dup
> > the return fd, and then add the duplicate FD back into the
> > fdset. This dance is overly verbose for the caller and
> > introduces extra failure modes which can be avoided by
> > folding all the logic into monitor_fdset_dup_fd_add and
> > removing monitor_fdset_get_fd entirely.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  include/monitor/monitor.h |  3 +-
> >  include/qemu/osdep.h      |  1 +
> >  monitor/misc.c            | 58 +++++++++++++++++----------------------
> >  stubs/fdset.c             |  8 ++----
> >  util/osdep.c              | 19 ++-----------
> >  5 files changed, 32 insertions(+), 57 deletions(-)
> >

> > -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
> > -{
> > -    MonFdset *mon_fdset;
> > -    MonFdsetFd *mon_fdset_fd_dup;
> > -
> > -    qemu_mutex_lock(&mon_fdsets_lock);
> > -    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
> > -        if (mon_fdset->id != fdset_id) {
> > -            continue;
> > +        if (fd == -1) {
> > +            errno = EINVAL;
> > +            return -1;
> 
> Missing qemu_mutex_unlock().
> 
> >          }
> 
> Old monitor_fdset_get_fd() returns -ENOENT when @fdset_id does not
> exist, and -EACCES when it doesn't contain a file descriptor matching
> @flags.
> 
> The new code seems to use EINVAL for the latter case.  Intentional?

No, its a mistake.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|