On Thu, Sep 03, 2020 at 10:52:40AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <[email protected]> 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é <[email protected]>
> > ---
> > 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 :|
