Re: [PATCH v3 06/10] monitor: release the lock before calling close()

["Alex Bennée" <alex.bennee@linaro.org>]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> As per comment, presumably to avoid syscall in critical section.
>
> Fixes: 0210c3b39bef08 ("monitor: Use LOCK_GUARD macros")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I know this is already merged but as an academic exercise we could have
kept the lock guard with a little restructuring like this:

  void qmp_closefd(const char *fdname, Error **errp)
  {
      Monitor *cur_mon = monitor_cur();
      mon_fd_t *monfd;
      int tmp_fd = -1;

      WITH_QEMU_LOCK_GUARD(&cur_mon->mon_lock) {
          QLIST_FOREACH(monfd, &cur_mon->fds, next) {
              if (strcmp(monfd->name, fdname) != 0) {
                  continue;
              }

              QLIST_REMOVE(monfd, next);
              tmp_fd = monfd->fd;
              g_free(monfd->name);
              g_free(monfd);
              break;
          }
      }

      if (tmp_fd > 0) {
          /* close() must be outside critical section */
          close(tmp_fd);
      } else {
          error_setg(errp, "File descriptor named '%s' not found", fdname);
      }
  }

To my mind it makes it easier to reason about locking but I probably
have an irrational aversion to multiple exit paths for locks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro