Hi
On Tue, Oct 31, 2023 at 1:06 PM Markus Armbruster <arm...@redhat.com> wrote:
>
> Markus Armbruster <arm...@redhat.com> writes:
>
> > Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> >
> >> Hi
> >>
> >> On Mon, Oct 30, 2023 at 5:37 PM Markus Armbruster <arm...@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 <arm...@redhat.com>
> >>
> >> Reviewed-by: Marc-André Lureau <marcandre.lur...@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?
Yes, I think it's safe and more future proof in general. Up to you if
you respin.
thanks
>
> >>> s->guest_note = NULL;
> >>> if (s->resume) {
> >>> if (s->detached) {
> >>> --
> >>> 2.41.0
> >>>
> >
> > Thanks!
>
