Re: [libvirt] [PATCH 08/17] conf: move chardev protocol parsing to separate function

On Mon, Aug 21, 2017 at 10:07:08AM +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/conf/domain_conf.c | 39 ++++++++++++++++++++++++++------------- > 1 file changed, 26 insertions(+), 13 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index bb4be5d1cd..8fe79f70bf 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -11040,7 +11063,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, > goto error; > } > protocolParsed = true; > - protocol = virXMLPropString(cur, "type"); > + if (virDomainChrSourceDefParseProtocol(def, cur) < 0) > + goto error; > } > } > > @@ -11151,16 +11175,6 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, > } > def->data.tcp.tlsFromConfig = !!tmp; > } > - > - if (!protocol) > - def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW; This removes the explicit assignment of VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW if no protocol node has been seen. The most direct equivalent would be: if (!protocolParsed) but I would also be okay with (in the order of preference) 1. initializing it before we start parsing the node

2. adding a _DEFAULT enum value and changing it in PostParse

3. explicitly assigning VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW = 0 and letting calloc do the initialization

ACK with that fixed