| Message ID | 20231030133712.2822276-3-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | dump: Minor fixes & improvements | expand |
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) > s->guest_note = NULL; > if (s->resume) { > if (s->detached) { > -- > 2.41.0 >
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. >> s->guest_note = NULL; >> if (s->resume) { >> if (s->detached) { >> -- >> 2.41.0 >> Thanks!
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!
Hi On Tue, Oct 31, 2023 at 1:06 PM Markus Armbruster <armbru@redhat.com> wrote: > > 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? 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! >
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); + } s->guest_note = NULL; if (s->resume) { if (s->detached) {
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> --- dump/dump.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)