Coverity reports that qemu_parse_config_group is returning without
unrefing the "crumpled" dictionary in case its top level item is a
list. But actually the contract with qemu_record_config_group is
the same as for qemu_parse_config_group itself: if those function
need to stash the dictionary they get, they have to take a reference
themselves (currently this is never the case for either function).
Therefore, just add an unconditional qobject_unref(crumpled) to
qemu_parse_config_group.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
softmmu/vl.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 2004d57108..7b54ddf6f4 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2196,9 +2196,10 @@ static void qemu_parse_config_group(const char *group, QDict *qdict,
if (qobject_type(crumpled) != QTYPE_QDICT) {
assert(qobject_type(crumpled) == QTYPE_QLIST);
error_setg(errp, "Lists cannot be at top level of a configuration section");
- return;
+ } else {
+ qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp);
}
- qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp);
+ qobject_unref(crumpled);
}
static void qemu_read_default_config_file(Error **errp)
--
2.31.1
Paolo Bonzini <pbonzini@redhat.com> writes: > Coverity reports that qemu_parse_config_group is returning without > unrefing the "crumpled" dictionary in case its top level item is a > list. But actually the contract with qemu_record_config_group is > the same as for qemu_parse_config_group itself: if those function > need to stash the dictionary they get, they have to take a reference > themselves (currently this is never the case for either function). > Therefore, just add an unconditional qobject_unref(crumpled) to > qemu_parse_config_group. > > Reported-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Do we want Fixes: c0d4aa82f895af67cbf7772324e05605e22b4162 here? > --- > softmmu/vl.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/softmmu/vl.c b/softmmu/vl.c > index 2004d57108..7b54ddf6f4 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -2196,9 +2196,10 @@ static void qemu_parse_config_group(const char *group, QDict *qdict, > if (qobject_type(crumpled) != QTYPE_QDICT) { > assert(qobject_type(crumpled) == QTYPE_QLIST); > error_setg(errp, "Lists cannot be at top level of a configuration section"); > - return; > + } else { > + qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp); > } > - qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp); > + qobject_unref(crumpled); > } > > static void qemu_read_default_config_file(Error **errp) Minimally invasive fix, but the result is a bit awkward. Possibly neater: if (qobject_type(crumpled) == QTYPE_QLIST) { error_setg(errp, "Lists cannot be at top level of a configuration section"); } else { assert(qobject_type(crumpled) == QTYPE_QDICT); qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp); } qobject_unref(crumpled); Regardless: Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 08/07/21 11:05, Markus Armbruster wrote: > Fixes: c0d4aa82f895af67cbf7772324e05605e22b4162 Good point, will add. Paolo
On 08/07/21 11:05, Markus Armbruster wrote: > Minimally invasive fix, but the result is a bit awkward. Possibly > neater: > > if (qobject_type(crumpled) == QTYPE_QLIST) { > error_setg(errp, > "Lists cannot be at top level of a configuration section"); > } else { > assert(qobject_type(crumpled) == QTYPE_QDICT); > qemu_record_config_group(group, qobject_to(QDict, crumpled), > false, errp); > } > qobject_unref(crumpled); Even better: switch (qobject_type(crumpled)) { case QTYPE_QDICT: qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp); break; case QTYPE_QLIST: error_setg(errp, "Lists cannot be at top level of a configuration section"); break; default: g_assert_unreachable(); } Paolo
© 2016 - 2024 Red Hat, Inc.