Re: [PATCH 2/5] dump: Fix g_array_unref(NULL) in dump-guest-memory

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
>> Hi
>>
>> On Mon, Oct 30, 2023 at 5:37 PM Markus Armbruster <armbru@redhat.com> wrote:
>>>
>>> When dump_init()'s check for non-zero @length fails, dump_cleanup()
>>> passes null s->string_table_buf to g_array_unref(), which spews "GLib:
>>> g_array_unref: assertion 'array' failed" to stderr.
>>>
>>> Guard the g_array_unref().
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>>> ---
>>>  dump/dump.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/dump/dump.c b/dump/dump.c
>>> index a1fad17f9c..d8ea364af2 100644
>>> --- a/dump/dump.c
>>> +++ b/dump/dump.c
>>> @@ -100,7 +100,9 @@ static int dump_cleanup(DumpState *s)
>>>      memory_mapping_list_free(&s->list);
>>>      close(s->fd);
>>>      g_free(s->guest_note);
>>> -    g_array_unref(s->string_table_buf);
>>> +    if (s->string_table_buf) {
>>> +        g_array_unref(s->string_table_buf);
>>> +    }
>>
>> or:
>> g_clear_pointer(&s->string_table_buf, g_array_unref)
>
> Since dump_cleanup() doesn't clear any of the other members of @s, I'll
> stick to g_array_unref() for consistency and simplicity.

Wait!  You suggest *unconditional*

         g_clear_pointer(&s->string_table_buf, g_array_unref)         

don't you?

Got a preference?

>>>      s->guest_note = NULL;
>>>      if (s->resume) {
>>>          if (s->detached) {
>>> --
>>> 2.41.0
>>>
>
> Thanks!