Re: [PATCH 04/25] util: validate return from xmlNodeGetContent before use

[Laine Stump]

On 6/25/20 6:55 PM, Ján Tomko wrote:

On a Wednesday in 2020, Laine Stump wrote:

There were a few uses of xmlNodeGetContent() that didn't check for
NULL before using the result.

A NULL return from xmlNodeGetContent() *could* (probably does) mean
that there was an Out of Memory condition, but it is unclear from the
documentation if that is always the case, or if it could just indicate
a missing value in the document, so we don't report an OOM error, but
just don't try to use it for, e.g., conversion to an integer.


Is it possible to have an element with "no value"?



I never found anywhere that said "No". But I also never found anywhere 
that says "yes", so I opted for "do no harm" (or something like that).




Even  gives me an empty string instead of NULL.



Okay, *that* says "No". So I'll change the patch to always report an OOM 
error.





Jano



Signed-off-by: Laine Stump 
---
src/conf/domain_conf.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)

Re: [PATCH 04/25] util: validate return from xmlNodeGetContent before use

[Ján Tomko]

On a Wednesday in 2020, Laine Stump wrote:

There were a few uses of xmlNodeGetContent() that didn't check for
NULL before using the result.

A NULL return from xmlNodeGetContent() *could* (probably does) mean
that there was an Out of Memory condition, but it is unclear from the
documentation if that is always the case, or if it could just indicate
a missing value in the document, so we don't report an OOM error, but
just don't try to use it for, e.g., conversion to an integer.


Is it possible to have an element with "no value"?

Even  gives me an empty string instead of NULL.

Jano



Signed-off-by: Laine Stump 
---
src/conf/domain_conf.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)


signature.asc
Description: PGP signature

[PATCH 04/25] util: validate return from xmlNodeGetContent before use

[Laine Stump]

There were a few uses of xmlNodeGetContent() that didn't check for
NULL before using the result.

A NULL return from xmlNodeGetContent() *could* (probably does) mean
that there was an Out of Memory condition, but it is unclear from the
documentation if that is always the case, or if it could just indicate
a missing value in the document, so we don't report an OOM error, but
just don't try to use it for, e.g., conversion to an integer.

Signed-off-by: Laine Stump 
---
 src/conf/domain_conf.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8cde1cd0e8..4d27c9caa8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10556,22 +10556,22 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
virXMLNodeNameEqual(cur, "wwn")) {
 wwn = (char *)xmlNodeGetContent(cur);
 
-if (!virValidateWWN(wwn))
+if (wwn && !virValidateWWN(wwn))
 goto error;
 } else if (!vendor &&
virXMLNodeNameEqual(cur, "vendor")) {
-vendor = (char *)xmlNodeGetContent(cur);
-
-if (strlen(vendor) > VENDOR_LEN) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("disk vendor is more than 8 characters"));
-goto error;
-}
+if ((vendor = (char *)xmlNodeGetContent(cur))) {
+if (strlen(vendor) > VENDOR_LEN) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("disk vendor is more than 8 characters"));
+goto error;
+}
 
-if (!virStringIsPrintable(vendor)) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("disk vendor is not printable string"));
-goto error;
+if (!virStringIsPrintable(vendor)) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("disk vendor is not printable string"));
+goto error;
+}
 }
 } else if (!product &&
virXMLNodeNameEqual(cur, "product")) {
@@ -20374,8 +20374,8 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
 
 if (STREQ_NULLABLE(tmp, "slic")) {
 VIR_FREE(tmp);
-tmp = virXMLNodeContentString(nodes[0]);
-def->os.slic_table = virFileSanitizePath(tmp);
+if ((tmp = virXMLNodeContentString(nodes[0])))
+def->os.slic_table = virFileSanitizePath(tmp);
 } else {
 virReportError(VIR_ERR_XML_ERROR,
_("Unknown acpi table type: %s"),
-- 
2.25.4