[libvirt] [PATCH 3/5] virDomainNetDefParseXML: suppress generation of MAC when VIR_DOMAIN_PARSE_NO_GENERATE is set

[Laine Stump]

On 03/17/2011 10:38 AM, Michal Privoznik wrote:
> ---
>   src/conf/domain_conf.c |   11 ++++++++---
>   1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c2c7057..7f9c178 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2491,8 +2491,7 @@ virDomainNetDefParseXML(virCapsPtr caps,
>       xmlNodePtr oldnode = ctxt->node;
>       int ret;
>
> -    if ((VIR_ALLOC(def)<  0) ||
> -        (VIR_ALLOC_N(def->mac,VIR_MAC_BUFLEN)<  0)) {
> +    if (VIR_ALLOC(def)<  0) {
>           virReportOOMError();
>           return NULL;
>       }
> @@ -2588,6 +2587,12 @@ virDomainNetDefParseXML(virCapsPtr caps,
>           cur = cur->next;
>       }
>
> +    if ((macaddr || !(flags&  VIR_DOMAIN_PARSE_NO_GENERATE))&&
> +        (VIR_ALLOC_N(def->mac, VIR_MAC_BUFLEN)<  0)) {
> +        virReportOOMError();
> +        goto error;
> +    }
> +
>       if (macaddr) {
>           if (virParseMacAddr((const char *)macaddr, def->mac)<  0) {
>               virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2595,7 +2600,7 @@ virDomainNetDefParseXML(virCapsPtr caps,
>                                    (const char *)macaddr);
>               goto error;
>           }
> -    } else {
> +    } else if (!(flags&  VIR_DOMAIN_PARSE_NO_GENERATE)) {
>           virCapabilitiesGenerateMac(caps, def->mac);

I'm going to voice my dislike of side-effects in Parse functions one 
last time (I think that a function called "...Parse" should interpret 
the data it's given, not add new data) and suggest that the Mac 
generation should always be done in the caller when needed rather than 
passing a strange flag down to the parser. (I know the parser was 
already generating the MAC; I think it should be changed to *not* do 
that) (or at the very least the flag shouldn't be "on == don't do 
something extra", it should be "on == do something extra").

However, if nobody else expresses agreement with me on that, I'll 
consider myself outvoted, and keep quiet during the next version of the 
patch (based on danpb's review of PATCH 4/5, it seems there is going to 
be another round anyway :-)

>       }
>