Peter Xu <pet...@redhat.com> writes:
> 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?
Yes, I'll use that.
>
> 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
>>
