[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 06/10] monitor: release the lock before calling close()
From: |
Alex Bennée |
Subject: |
Re: [PATCH v3 06/10] monitor: release the lock before calling close() |
Date: |
Thu, 02 Mar 2023 09:34:15 +0000 |
User-agent: |
mu4e 1.9.21; emacs 29.0.60 |
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
- Re: [PATCH v3 06/10] monitor: release the lock before calling close(),
Alex Bennée <=