[PATCH 06/12] lib: Almost eliminate use of virTristateBoolTypeFromString()

[Ján Tomko]

On a Friday in 2022, Michal Privoznik wrote:
>There are couple of places where virTristateBoolTypeFromString()
>is called. Well, the same result can be achieved by
>virXMLPropTristateBool() and on fewer lines.
>
>Note there are couple of places left untouched because those
>don't care about error reporting and thus are shorter they way
>they are now.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/conf/cpu_conf.c            |  20 ++--
> src/conf/domain_conf.c         | 170 ++++++++++++---------------------
> src/conf/domain_conf.h         |  13 ++-
> src/conf/network_conf.c        | 116 ++++++----------------
> src/conf/network_conf.h        |  12 +--
> src/conf/storage_conf.c        |  17 ++--
> src/conf/storage_source_conf.c |  14 +--
> src/conf/storage_source_conf.h |   2 +-
> src/conf/virnetworkportdef.c   |  31 +++---
> src/conf/virnetworkportdef.h   |   4 +-
> src/cpu/cpu_x86.c              |  23 ++---
> src/qemu/qemu_capabilities.c   |  28 ++----
> src/qemu/qemu_domain.c         |  16 +---
> 13 files changed, 157 insertions(+), 309 deletions(-)
>
>diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
>index 4d61bfd01b..8e61a16919 100644
>--- a/src/conf/cpu_conf.c
>+++ b/src/conf/cpu_conf.c
>@@ -16972,18 +16930,14 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
>     }
>
>     if ((node = virXPathNode("./os/bootmenu[1]", ctxt))) {
>-        tmp = virXMLPropString(node, "enable");
>-        if (tmp) {
>-            def->os.bootmenu = virTristateBoolTypeFromString(tmp);
>-            if (def->os.bootmenu <= 0) {
>-                /* In order not to break misconfigured machines, this
>-                 * should not emit an error, but rather set the bootmenu
>-                 * to disabled */
>-                VIR_WARN("disabling bootmenu due to unknown option '%s'",
>-                         tmp);
>-                def->os.bootmenu = VIR_TRISTATE_BOOL_NO;
>-            }
>-            VIR_FREE(tmp);
>+        if (virXMLPropTristateBool(node, "enable",
>+                                   VIR_XML_PROP_NONE,
>+                                   &def->os.bootmenu) < 0) {
>+            /* In order not to break misconfigured machines, this
>+             * should not emit an error, but rather set the bootmenu
>+             * to disabled */

The comment is now incorrect - virXMLPropTristateBool would log an
error.

Introduced by commit 4f24ca01e881ec8df69861a3386b697ff528d3e3
CommitDate: 2010-07-27 16:38:32 -0400

     qemu: Allow setting boot menu on/off

we accepted a value of "yes" meaning "yes", and all other values meant
"no".

This was later refactored by
commit 8c952908681ca66ba4be75362de7e593d6c44f94
CommitDate: 2012-09-20 10:59:35 +0200
     qemu: Cleanup boot parameter building

to use an enum with "yes" and "no", but no error checking.


I think we can safely return an error here after all the years.

Jano

>+            VIR_WARN("disabling bootmenu due to unknown option");
>+            def->os.bootmenu = VIR_TRISTATE_BOOL_NO;
>         }
>
>         tmp = virXMLPropString(node, "timeout");
-------------- 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/20220121/6ba7e4da/attachment-0001.sig>