On Thu, May 23, 2024 at 04:05:35PM -0300, Fabiano Rosas wrote:
> Introduce two new functions to remove and free no longer used fds and
> fdsets.
>
> We need those to decouple the remove/free routines from
> monitor_fdset_cleanup() which will go away in the next patches.
>
> The new functions:
>
> - monitor_fdset_free() will be used when a monitor connection closes
> and when an fd is removed to cleanup any fdset that is now empty.
>
> - monitor_fdset_fd_free() will be used to remove one or more fds that
> have been explicitly targeted by qmp_remove_fd().
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
Reviewed-by: Peter Xu <pet...@redhat.com>
One nitpick below.
> ---
> monitor/fds.c | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/monitor/fds.c b/monitor/fds.c
> index fb9f58c056..101e21720d 100644
> --- a/monitor/fds.c
> +++ b/monitor/fds.c
> @@ -167,6 +167,22 @@ int monitor_get_fd(Monitor *mon, const char *fdname,
> Error **errp)
> return -1;
> }
>
> +static void monitor_fdset_free(MonFdset *mon_fdset)
> +{
> + if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) {
> + QLIST_REMOVE(mon_fdset, next);
> + g_free(mon_fdset);
> + }
> +}
Would monitor_fdset_free_if_empty() (or similar) slightly better?
static void monitor_fdset_free(MonFdset *mon_fdset)
{
QLIST_REMOVE(mon_fdset, next);
g_free(mon_fdset);
}
static void monitor_fdset_free_if_empty(MonFdset *mon_fdset)
{
if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) {
monitor_fdset_free(mon_fdset);
}
}
> +
> +static void monitor_fdset_fd_free(MonFdsetFd *mon_fdset_fd)
> +{
> + close(mon_fdset_fd->fd);
> + g_free(mon_fdset_fd->opaque);
> + QLIST_REMOVE(mon_fdset_fd, next);
> + g_free(mon_fdset_fd);
> +}
> +
> static void monitor_fdset_cleanup(MonFdset *mon_fdset)
> {
> MonFdsetFd *mon_fdset_fd;
> @@ -176,17 +192,11 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset)
> if ((mon_fdset_fd->removed ||
> (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) &&
> runstate_is_running()) {
> - close(mon_fdset_fd->fd);
> - g_free(mon_fdset_fd->opaque);
> - QLIST_REMOVE(mon_fdset_fd, next);
> - g_free(mon_fdset_fd);
> + monitor_fdset_fd_free(mon_fdset_fd);
> }
> }
>
> - if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) {
> - QLIST_REMOVE(mon_fdset, next);
> - g_free(mon_fdset);
> - }
> + monitor_fdset_free(mon_fdset);
> }
>
> void monitor_fdsets_cleanup(void)
> --
> 2.35.3
>
--
Peter Xu
