Re: [RFC 08/21] conf: Generate virNetworkDNSTxtDefFormatBuf

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

On Wed, Jun 10, 2020 at 09:20:36AM +0800, Shi Lei wrote:
> Signed-off-by: Shi Lei <shi_lei@xxxxxxxxxxxxxx>
> ---
>  src/conf/network_conf.c | 4 ++--
>  src/conf/network_conf.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 964a8a7..b807bac 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -2280,8 +2280,8 @@ virNetworkDNSDefFormat(virBufferPtr buf,
>      }
>  
>      for (i = 0; i < def->ntxts; i++) {
> -        virBufferEscapeString(buf, "<txt name='%s' ", def->txts[i].name);
> -        virBufferEscapeString(buf, "value='%s'/>\n", def->txts[i].value);
> +        if (virNetworkDNSTxtDefFormatBuf(buf, "txt", &def->txts[i], NULL) < 0)
> +            return -1;
>      }

For sake of review, the new code looks like this:

int
virNetworkDNSTxtDefFormatBuf(virBufferPtr buf,
                             const char *name,
                             const virNetworkDNSTxtDef *def,
                             void *opaque)
{
    VIR_USED(opaque);

    if (!def)
        return 0;

    if (!(def->name || def->value))
        return 0;

    virBufferAsprintf(buf, "<%s", name);

    if (def->name)
        virBufferAsprintf(buf, " name='%s'", def->name);

    if (def->value)
        virBufferAsprintf(buf, " value='%s'", def->value);

    virBufferAddLit(buf, "/>\n");

    return 0;
}

This is a lot longer, but obviously the code is more general
purpose.

I'm not sure why we need to pass "txt" into this method though.
Can't we just hardcode "<txt" instead of formatting "<%s", name ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|