[libvirt PATCH v6 0/8] Refactor XML parsing boilerplate code

Tim Wiederhake posted 8 patches 3 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210416094152.202334-1-twiederh@redhat.com
src/conf/cpu_conf.c      |  14 +--
src/conf/domain_conf.c   |  14 +--
src/conf/network_conf.c  |  16 +--
src/libvirt_private.syms |   5 +
src/util/virxml.c        | 249 +++++++++++++++++++++++++++++++++++++++
src/util/virxml.h        |  42 +++++++
6 files changed, 305 insertions(+), 35 deletions(-)
[libvirt PATCH v6 0/8] Refactor XML parsing boilerplate code
Posted by Tim Wiederhake 3 years ago
This series lays the groundwork for replacing some recurring boilerplate code
in src/conf/ regarding the extraction of XML attribute values.

For an on / off attribute, the boilerplate code looks roughly like this,

  g_autofree char *str = NULL;
  if (str = virXMLPropString(node, "x")) {
    int val;
    if ((val = virTristateBoolTypeFromString(str)) <= 0) {
      virReportError(...)
      return -1;
    }
    def->x = val;
  }

with some variations regarding how `str` is free'd in case of later re-use,
the exact error message for invalid values, whether or not
`VIR_TRISTATE_(SWITCH|BOOL)_ABSENT` is explicitly checked for (`val < 0` vs.
`val <= 0`), whether an intermediate variable is used or the value is assigned
directly, and in some cases the conditions in the two if-blocks are merged.

After the refactoring, the above code block looks like this:

  if (virXMLPropTristateBool(node, "x", VIR_XML_PROP_OPTIONAL, &def->x) < 0)
      return -1;

Similar functions are introduced for integer valued attributes, unsigned
integer valued attributes and enum valued attributes.

Patches #6, #7, and #8 demonstrate the application of these function and stand
representative of more patches that I did not sent along yet as to not drown
the mailing list in spam. These patches remove a total of ~ 1000 lines of code
and fix some places, where e.g. `<foo bar="default"/>` would incorrectly be
accepted as virXMLTristateBool.

As an added benefit, this refactoring makes the error messages for invalid
values in these XML attributes much more consistent:

  $ git diff master | grep "^-.*_(\"" | wc -l
  239
  $ git diff master | grep "^+.*_(\"" | wc -l
  21

V1: https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html
V2: https://listman.redhat.com/archives/libvir-list/2021-March/msg00994.html
V3: https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html
V4: https://listman.redhat.com/archives/libvir-list/2021-April/msg00209.html
V5: https://listman.redhat.com/archives/libvir-list/2021-April/msg00232.html

Changes since V5:
* Applied changes requested in
  https://listman.redhat.com/archives/libvir-list/2021-April/msg00658.html

Cheers,
Tim

Tim Wiederhake (8):
  virxml: Add virXMLPropTristateBool
  virxml: Add virXMLPropTristateSwitch
  virxml: Add virXMLPropInt
  virxml: Add virXMLPropUInt
  virxml: Add virXMLPropEnum
  virNetworkForwardNatDefParseXML: Use virXMLProp*
  virDomainIOThreadIDDefParseXML: Use virXMLProp*
  virCPUDefParseXML: Use virXMLProp*

 src/conf/cpu_conf.c      |  14 +--
 src/conf/domain_conf.c   |  14 +--
 src/conf/network_conf.c  |  16 +--
 src/libvirt_private.syms |   5 +
 src/util/virxml.c        | 249 +++++++++++++++++++++++++++++++++++++++
 src/util/virxml.h        |  42 +++++++
 6 files changed, 305 insertions(+), 35 deletions(-)

-- 
2.26.2


Re: [libvirt PATCH v6 0/8] Refactor XML parsing boilerplate code
Posted by Peter Krempa 3 years ago
On Fri, Apr 16, 2021 at 11:41:44 +0200, Tim Wiederhake wrote:

[...]

> Tim Wiederhake (8):
>   virxml: Add virXMLPropTristateBool
>   virxml: Add virXMLPropTristateSwitch
>   virxml: Add virXMLPropInt
>   virxml: Add virXMLPropUInt
>   virxml: Add virXMLPropEnum

I've rebased these on top of the virxml.h formatting fix and pushed them
now. Additionally I've converted few types to appropriate enum values to
see how compilers will like that in respect to virXMLPropEnum:

https://listman.redhat.com/archives/libvir-list/2021-April/msg00679.html

>   virNetworkForwardNatDefParseXML: Use virXMLProp*
>   virDomainIOThreadIDDefParseXML: Use virXMLProp*
>   virCPUDefParseXML: Use virXMLProp*
> 
>  src/conf/cpu_conf.c      |  14 +--
>  src/conf/domain_conf.c   |  14 +--
>  src/conf/network_conf.c  |  16 +--
>  src/libvirt_private.syms |   5 +
>  src/util/virxml.c        | 249 +++++++++++++++++++++++++++++++++++++++
>  src/util/virxml.h        |  42 +++++++
>  6 files changed, 305 insertions(+), 35 deletions(-)
> 
> -- 
> 2.26.2
> 
>