src/conf/domain_event.c | 2 +- tests/vboxsnapshotxmltest.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
g_regex_unref reports an error if called with a NULL argument.
We have two cases in the code where we (possibly) call it on a NULL
argument. The interesting one is in virDomainQemuMonitorEventCleanup.
Based on VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX, we unref
data->regex, which has two problems:
* On the client side, flags is -1 so the comparison is true even if no
regex was used, reproducible by:
$ virsh qemu-monitor-event --timeout 1
which results in an ugly error:
(process:1289846): GLib-CRITICAL **: 14:58:42.631: g_regex_unref: assertion 'regex != NULL' failed
* On the server side, we only create the regex if both the flag and the
string are present, so it's possible to trigger this message by:
$ virsh qemu-monitor-event --regex --timeout 1
Use a non-NULL comparison instead of the flag to decide whether we need
to unref the regex.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 71efb59a4de7c51b1bc889a316f1796ebf55738f
https://bugzilla.redhat.com/show_bug.cgi?id=1861176
---
src/conf/domain_event.c | 2 +-
tests/vboxsnapshotxmltest.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 33fbf10406..d3acde0236 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -2194,7 +2194,7 @@ virDomainQemuMonitorEventCleanup(void *opaque)
virDomainQemuMonitorEventData *data = opaque;
VIR_FREE(data->event);
- if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX)
+ if (data->regex)
g_regex_unref(data->regex);
if (data->freecb)
(data->freecb)(data->opaque);
diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c
index 7e3f85cc58..d2beb7858d 100644
--- a/tests/vboxsnapshotxmltest.c
+++ b/tests/vboxsnapshotxmltest.c
@@ -135,7 +135,8 @@ mymain(void)
DO_TEST("2disks-3snap-brother");
cleanup:
- g_regex_unref(testSnapshotXMLVariableLineRegex);
+ if (testSnapshotXMLVariableLineRegex)
+ g_regex_unref(testSnapshotXMLVariableLineRegex);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.26.2
On Tue, Sep 08, 2020 at 03:20:52PM +0200, Ján Tomko wrote: >g_regex_unref reports an error if called with a NULL argument. > >We have two cases in the code where we (possibly) call it on a NULL >argument. The interesting one is in virDomainQemuMonitorEventCleanup. > >Based on VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX, we unref >data->regex, which has two problems: > >* On the client side, flags is -1 so the comparison is true even if no > regex was used, reproducible by: > $ virsh qemu-monitor-event --timeout 1 > which results in an ugly error: >(process:1289846): GLib-CRITICAL **: 14:58:42.631: g_regex_unref: assertion 'regex != NULL' failed >* On the server side, we only create the regex if both the flag and the > string are present, so it's possible to trigger this message by: > $ virsh qemu-monitor-event --regex --timeout 1 > >Use a non-NULL comparison instead of the flag to decide whether we need >to unref the regex. > >Signed-off-by: Ján Tomko <jtomko@redhat.com> >Fixes: 71efb59a4de7c51b1bc889a316f1796ebf55738f >https://bugzilla.redhat.com/show_bug.cgi?id=1861176 Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Any reason why regex_unref does not accept NULLs? It caught a misuse in the first case, but the second one could just make the code cleaner. Maybe we could create our own auto type over it? =D
On Tue, Sep 08, 2020 at 15:20:52 +0200, Ján Tomko wrote: > g_regex_unref reports an error if called with a NULL argument. > > We have two cases in the code where we (possibly) call it on a NULL > argument. The interesting one is in virDomainQemuMonitorEventCleanup. > > Based on VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX, we unref > data->regex, which has two problems: > > * On the client side, flags is -1 so the comparison is true even if no > regex was used, reproducible by: > $ virsh qemu-monitor-event --timeout 1 > which results in an ugly error: > (process:1289846): GLib-CRITICAL **: 14:58:42.631: g_regex_unref: assertion 'regex != NULL' failed > * On the server side, we only create the regex if both the flag and the > string are present, so it's possible to trigger this message by: > $ virsh qemu-monitor-event --regex --timeout 1 > > Use a non-NULL comparison instead of the flag to decide whether we need > to unref the regex. Please modify this last sentence to describe both fixes. I've flushed out that there's another case by the time I've read the end here. > Signed-off-by: Ján Tomko <jtomko@redhat.com> > Fixes: 71efb59a4de7c51b1bc889a316f1796ebf55738f > https://bugzilla.redhat.com/show_bug.cgi?id=1861176 > --- > src/conf/domain_event.c | 2 +- > tests/vboxsnapshotxmltest.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Peter Krempa <pkrempa@redhat.com>
© 2016 - 2024 Red Hat, Inc.