monitor/misc.c | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-)
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Use the lock guard macros in monitor/misc.c - saves
a lot of unlocks in error paths, and the occasional goto.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
--
v2:
The file changed quite a bit since my original posting, so reworked.
---
monitor/misc.c | 44 ++++++++++++++------------------------------
1 file changed, 14 insertions(+), 30 deletions(-)
diff --git a/monitor/misc.c b/monitor/misc.c
index 0b1b9b196c..9df917f464 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -141,13 +141,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
handle_hmp_command(&hmp, command_line);
cur_mon = old_mon;
- qemu_mutex_lock(&hmp.common.mon_lock);
- if (qstring_get_length(hmp.common.outbuf) > 0) {
- output = g_strdup(qstring_get_str(hmp.common.outbuf));
- } else {
- output = g_strdup("");
+ WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) {
+ if (qstring_get_length(hmp.common.outbuf) > 0) {
+ output = g_strdup(qstring_get_str(hmp.common.outbuf));
+ } else {
+ output = g_strdup("");
+ }
}
- qemu_mutex_unlock(&hmp.common.mon_lock);
out:
monitor_data_destroy(&hmp.common);
@@ -1248,7 +1248,7 @@ void qmp_getfd(const char *fdname, Error **errp)
return;
}
- qemu_mutex_lock(&cur_mon->mon_lock);
+ QEMU_LOCK_GUARD(&cur_mon->mon_lock);
QLIST_FOREACH(monfd, &cur_mon->fds, next) {
if (strcmp(monfd->name, fdname) != 0) {
continue;
@@ -1256,7 +1256,6 @@ void qmp_getfd(const char *fdname, Error **errp)
tmp_fd = monfd->fd;
monfd->fd = fd;
- qemu_mutex_unlock(&cur_mon->mon_lock);
/* Make sure close() is outside critical section */
close(tmp_fd);
return;
@@ -1267,7 +1266,6 @@ void qmp_getfd(const char *fdname, Error **errp)
monfd->fd = fd;
QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
- qemu_mutex_unlock(&cur_mon->mon_lock);
}
void qmp_closefd(const char *fdname, Error **errp)
@@ -1299,7 +1297,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
{
mon_fd_t *monfd;
- qemu_mutex_lock(&mon->mon_lock);
+ QEMU_LOCK_GUARD(&mon->mon_lock);
QLIST_FOREACH(monfd, &mon->fds, next) {
int fd;
@@ -1313,12 +1311,10 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
QLIST_REMOVE(monfd, next);
g_free(monfd->name);
g_free(monfd);
- qemu_mutex_unlock(&mon->mon_lock);
return fd;
}
- qemu_mutex_unlock(&mon->mon_lock);
error_setg(errp, "File descriptor named '%s' has not been found", fdname);
return -1;
}
@@ -1350,11 +1346,10 @@ void monitor_fdsets_cleanup(void)
MonFdset *mon_fdset;
MonFdset *mon_fdset_next;
- qemu_mutex_lock(&mon_fdsets_lock);
+ QEMU_LOCK_GUARD(&mon_fdsets_lock);
QLIST_FOREACH_SAFE(mon_fdset, &mon_fdsets, next, mon_fdset_next) {
monitor_fdset_cleanup(mon_fdset);
}
- qemu_mutex_unlock(&mon_fdsets_lock);
}
AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
@@ -1389,7 +1384,7 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
MonFdsetFd *mon_fdset_fd;
char fd_str[60];
- qemu_mutex_lock(&mon_fdsets_lock);
+ QEMU_LOCK_GUARD(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
if (mon_fdset->id != fdset_id) {
continue;
@@ -1409,12 +1404,10 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
goto error;
}
monitor_fdset_cleanup(mon_fdset);
- qemu_mutex_unlock(&mon_fdsets_lock);
return;
}
error:
- qemu_mutex_unlock(&mon_fdsets_lock);
if (has_fd) {
snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64,
fdset_id, fd);
@@ -1430,7 +1423,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
MonFdsetFd *mon_fdset_fd;
FdsetInfoList *fdset_list = NULL;
- qemu_mutex_lock(&mon_fdsets_lock);
+ QEMU_LOCK_GUARD(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info));
FdsetFdInfoList *fdsetfd_list = NULL;
@@ -1460,7 +1453,6 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
fdset_info->next = fdset_list;
fdset_list = fdset_info;
}
- qemu_mutex_unlock(&mon_fdsets_lock);
return fdset_list;
}
@@ -1554,7 +1546,7 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags)
#else
MonFdset *mon_fdset;
- qemu_mutex_lock(&mon_fdsets_lock);
+ QEMU_LOCK_GUARD(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
MonFdsetFd *mon_fdset_fd;
MonFdsetFd *mon_fdset_fd_dup;
@@ -1569,7 +1561,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags)
QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL);
if (mon_fd_flags == -1) {
- qemu_mutex_unlock(&mon_fdsets_lock);
return -1;
}
@@ -1580,25 +1571,21 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags)
}
if (fd == -1) {
- qemu_mutex_unlock(&mon_fdsets_lock);
errno = EACCES;
return -1;
}
dup_fd = qemu_dup_flags(fd, flags);
if (dup_fd == -1) {
- qemu_mutex_unlock(&mon_fdsets_lock);
return -1;
}
mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));
mon_fdset_fd_dup->fd = dup_fd;
QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next);
- qemu_mutex_unlock(&mon_fdsets_lock);
return dup_fd;
}
- qemu_mutex_unlock(&mon_fdsets_lock);
errno = ENOENT;
return -1;
#endif
@@ -1609,7 +1596,7 @@ static int64_t monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd_dup;
- qemu_mutex_lock(&mon_fdsets_lock);
+ QEMU_LOCK_GUARD(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {
if (mon_fdset_fd_dup->fd == dup_fd) {
@@ -1619,17 +1606,14 @@ static int64_t monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
if (QLIST_EMPTY(&mon_fdset->dup_fds)) {
monitor_fdset_cleanup(mon_fdset);
}
- goto err;
+ return -1;
} else {
- qemu_mutex_unlock(&mon_fdsets_lock);
return mon_fdset->id;
}
}
}
}
-err:
- qemu_mutex_unlock(&mon_fdsets_lock);
return -1;
}
--
2.26.2
* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Use the lock guard macros in monitor/misc.c - saves > a lot of unlocks in error paths, and the occasional goto. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Queued > -- > v2: > The file changed quite a bit since my original posting, so reworked. > > --- > monitor/misc.c | 44 ++++++++++++++------------------------------ > 1 file changed, 14 insertions(+), 30 deletions(-) > > diff --git a/monitor/misc.c b/monitor/misc.c > index 0b1b9b196c..9df917f464 100644 > --- a/monitor/misc.c > +++ b/monitor/misc.c > @@ -141,13 +141,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, > handle_hmp_command(&hmp, command_line); > cur_mon = old_mon; > > - qemu_mutex_lock(&hmp.common.mon_lock); > - if (qstring_get_length(hmp.common.outbuf) > 0) { > - output = g_strdup(qstring_get_str(hmp.common.outbuf)); > - } else { > - output = g_strdup(""); > + WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) { > + if (qstring_get_length(hmp.common.outbuf) > 0) { > + output = g_strdup(qstring_get_str(hmp.common.outbuf)); > + } else { > + output = g_strdup(""); > + } > } > - qemu_mutex_unlock(&hmp.common.mon_lock); > > out: > monitor_data_destroy(&hmp.common); > @@ -1248,7 +1248,7 @@ void qmp_getfd(const char *fdname, Error **errp) > return; > } > > - qemu_mutex_lock(&cur_mon->mon_lock); > + QEMU_LOCK_GUARD(&cur_mon->mon_lock); > QLIST_FOREACH(monfd, &cur_mon->fds, next) { > if (strcmp(monfd->name, fdname) != 0) { > continue; > @@ -1256,7 +1256,6 @@ void qmp_getfd(const char *fdname, Error **errp) > > tmp_fd = monfd->fd; > monfd->fd = fd; > - qemu_mutex_unlock(&cur_mon->mon_lock); > /* Make sure close() is outside critical section */ > close(tmp_fd); > return; > @@ -1267,7 +1266,6 @@ void qmp_getfd(const char *fdname, Error **errp) > monfd->fd = fd; > > QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next); > - qemu_mutex_unlock(&cur_mon->mon_lock); > } > > void qmp_closefd(const char *fdname, Error **errp) > @@ -1299,7 +1297,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) > { > mon_fd_t *monfd; > > - qemu_mutex_lock(&mon->mon_lock); > + QEMU_LOCK_GUARD(&mon->mon_lock); > QLIST_FOREACH(monfd, &mon->fds, next) { > int fd; > > @@ -1313,12 +1311,10 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) > QLIST_REMOVE(monfd, next); > g_free(monfd->name); > g_free(monfd); > - qemu_mutex_unlock(&mon->mon_lock); > > return fd; > } > > - qemu_mutex_unlock(&mon->mon_lock); > error_setg(errp, "File descriptor named '%s' has not been found", fdname); > return -1; > } > @@ -1350,11 +1346,10 @@ void monitor_fdsets_cleanup(void) > MonFdset *mon_fdset; > MonFdset *mon_fdset_next; > > - qemu_mutex_lock(&mon_fdsets_lock); > + QEMU_LOCK_GUARD(&mon_fdsets_lock); > QLIST_FOREACH_SAFE(mon_fdset, &mon_fdsets, next, mon_fdset_next) { > monitor_fdset_cleanup(mon_fdset); > } > - qemu_mutex_unlock(&mon_fdsets_lock); > } > > AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque, > @@ -1389,7 +1384,7 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) > MonFdsetFd *mon_fdset_fd; > char fd_str[60]; > > - qemu_mutex_lock(&mon_fdsets_lock); > + QEMU_LOCK_GUARD(&mon_fdsets_lock); > QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { > if (mon_fdset->id != fdset_id) { > continue; > @@ -1409,12 +1404,10 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) > goto error; > } > monitor_fdset_cleanup(mon_fdset); > - qemu_mutex_unlock(&mon_fdsets_lock); > return; > } > > error: > - qemu_mutex_unlock(&mon_fdsets_lock); > if (has_fd) { > snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64, > fdset_id, fd); > @@ -1430,7 +1423,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) > MonFdsetFd *mon_fdset_fd; > FdsetInfoList *fdset_list = NULL; > > - qemu_mutex_lock(&mon_fdsets_lock); > + QEMU_LOCK_GUARD(&mon_fdsets_lock); > QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { > FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info)); > FdsetFdInfoList *fdsetfd_list = NULL; > @@ -1460,7 +1453,6 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) > fdset_info->next = fdset_list; > fdset_list = fdset_info; > } > - qemu_mutex_unlock(&mon_fdsets_lock); > > return fdset_list; > } > @@ -1554,7 +1546,7 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags) > #else > MonFdset *mon_fdset; > > - qemu_mutex_lock(&mon_fdsets_lock); > + QEMU_LOCK_GUARD(&mon_fdsets_lock); > QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { > MonFdsetFd *mon_fdset_fd; > MonFdsetFd *mon_fdset_fd_dup; > @@ -1569,7 +1561,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags) > QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { > mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL); > if (mon_fd_flags == -1) { > - qemu_mutex_unlock(&mon_fdsets_lock); > return -1; > } > > @@ -1580,25 +1571,21 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags) > } > > if (fd == -1) { > - qemu_mutex_unlock(&mon_fdsets_lock); > errno = EACCES; > return -1; > } > > dup_fd = qemu_dup_flags(fd, flags); > if (dup_fd == -1) { > - qemu_mutex_unlock(&mon_fdsets_lock); > return -1; > } > > mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup)); > mon_fdset_fd_dup->fd = dup_fd; > QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next); > - qemu_mutex_unlock(&mon_fdsets_lock); > return dup_fd; > } > > - qemu_mutex_unlock(&mon_fdsets_lock); > errno = ENOENT; > return -1; > #endif > @@ -1609,7 +1596,7 @@ static int64_t monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove) > MonFdset *mon_fdset; > MonFdsetFd *mon_fdset_fd_dup; > > - qemu_mutex_lock(&mon_fdsets_lock); > + QEMU_LOCK_GUARD(&mon_fdsets_lock); > QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { > QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) { > if (mon_fdset_fd_dup->fd == dup_fd) { > @@ -1619,17 +1606,14 @@ static int64_t monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove) > if (QLIST_EMPTY(&mon_fdset->dup_fds)) { > monitor_fdset_cleanup(mon_fdset); > } > - goto err; > + return -1; > } else { > - qemu_mutex_unlock(&mon_fdsets_lock); > return mon_fdset->id; > } > } > } > } > > -err: > - qemu_mutex_unlock(&mon_fdsets_lock); > return -1; > } > > -- > 2.26.2 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 9/24/20 11:17 AM, Dr. David Alan Gilbert wrote: > * Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote: >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> >> >> Use the lock guard macros in monitor/misc.c - saves >> a lot of unlocks in error paths, and the occasional goto. >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Queued Sorry I had this tagged but forgot: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
© 2016 - 2024 Red Hat, Inc.