On Wed, May 23, 2018 at 10:39:20AM +0200, Markus Armbruster wrote:
> Peter Xu <pet...@redhat.com> writes:
>
> > On Fri, May 18, 2018 at 02:27:00PM +0200, Markus Armbruster wrote:
> >> Peter Xu <pet...@redhat.com> writes:
> >>
> >> > On Thu, May 17, 2018 at 03:03:02PM +0200, Markus Armbruster wrote:
> >> >
> >> > [...]
> >> >
> >> >> > @@ -2502,7 +2525,9 @@ int monitor_fdset_get_fd(int64_t fdset_id, int
> >> >> > flags)
> >> >> > MonFdset *mon_fdset;
> >> >> > MonFdsetFd *mon_fdset_fd;
> >> >> > int mon_fd_flags;
> >> >> > + int ret = -1;
> >> >>
> >> >> Suggest not to initialize ret, and instead ret = -1 on both failure
> >> >> paths.
> >> >
> >> > [1]
> >> >
> >> > But there is a third hidden failure path that we failed to find the fd
> >> > specified? In that case we still need that initial value.
> >>
> >> You're right. However, that failure path could be made explicit easily:
> >>
> >> QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
> >> [got out on error and on finding the right one...]
> >> }
> >> ret = -1;
> >> errno = ENOENT;
> >>
> >> out:
> >> qemu_mutex_unlock(&mon_fdsets_lock);
> >> return ret;
> >>
> >> I find this clearer. Your choice.
> >
> > Yes this works too. Considering that I just posted v6, I'll
> > temporarily just keep the old way.
>
> Your v6 raced with my review of v5. Do you intend to post v7? If not,
> I need to figure out what I can and want to do to v6 on commit to my
> tree.
I can repost v7 after we finish the discussion in the other thread:
[PATCH v5 3/4] monitor: more comments on lock-free fleids/funcs
Message-ID: <87muwqixla....@dusky.pond.sub.org>
I think there is a comment to be refined, meanwhile I can at least
pick up the qemu_open() suggestion too.
Regards,
--
Peter Xu
