[PATCH 03/25] conf: refactor virDomainBlkioDeviceParseXML to remove possible NULL dereference
Ján Tomko
jtomko at redhat.com
Thu Jun 25 22:44:00 UTC 2020
On a Wednesday in 2020, Laine Stump wrote:
>virDomainBlkioDeviceParseXML() has multiple cases of sending the
>return from xmlNodeGetContent() directly to virStrToLong_xx() without
>checking for NULL. Although it is *very* rare for xmlNodeGetContent()
>to return NULL (possibly it only happens in an OOM condition? The
>documentation is unclear), it could happen, and the refactor in this
>patch manages to eliminate several lines of repeated code while adding
>in a (single) check for NULL.
>
>Signed-off-by: Laine Stump <laine at redhat.com>
>---
> src/conf/domain_conf.c | 39 +++++++++++++++------------------------
> 1 file changed, 15 insertions(+), 24 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 1916b51d38..8cde1cd0e8 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -1628,73 +1628,64 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root,
> virBlkioDevicePtr dev)
> {
> xmlNodePtr node;
>- g_autofree char *c = NULL;
>+ g_autofree char *path = NULL;
>
> node = root->children;
> while (node) {
>- if (node->type == XML_ELEMENT_NODE) {
>- if (virXMLNodeNameEqual(node, "path") && !dev->path) {
>- dev->path = (char *)xmlNodeGetContent(node);
>+ g_autofree char *c = NULL;
>+
>+ if (node->type == XML_ELEMENT_NODE &&
>+ (c = (char *)xmlNodeGetContent(node))) {
Missing ErrorReport if xmlNodeGetContent fails.
Converting this open-coded for loop to an actual for loop would
grant us 'continue' privileges, which would make the checks
nicer and give a possibility of assigning the path directly
to 'path', without the extra steal_pointer.
Alternatively, the minimum diff where I'd consider this patch to be
a strict improvement is:
} else if (node->type == XML_ELEMENT_NODE && !c) {
virReportOOMError();
return -1;
}
With that: Reviewed-by: Ján Tomko <jtomko at redhat.com>
Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200626/95a7ce0e/attachment-0001.sig>
More information about the libvir-list
mailing list