[libvirt] [PATCHv2 12/13] snapshot: minor cleanups from reviewing indentation

[Peter Krempa]

Dňa 29.9.2011 18:22, Eric Blake  wrote / napísal(a):
> Break some long lines, and use more efficient functions when possible,
> such as relying on virBufferEscapeString to skip output on a NULL arg.
> Ensure that output does not embed newlines, since auto-indent won't
> work in those situations.
>
> * src/conf/domain_conf.c (virDomainTimerDefFormat): Break output lines.
> (virDomainDefFormatInternal, virDomainDiskDefFormat)
> (virDomainActualNetDefFormat, virDomainNetDefFormat)
> (virDomainHostdevDefFormat): Minor cleanups.
> ---
>   src/conf/domain_conf.c |  179 ++++++++++++++++++++++--------------------------
>   1 files changed, 81 insertions(+), 98 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6ab73ec..1871974 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9476,15 +9470,13 @@ virDomainNetDefFormat(virBufferPtr buf,
>           break;
>
>       case VIR_DOMAIN_NET_TYPE_ETHERNET:
> -        if (def->data.ethernet.dev)
> -            virBufferEscapeString(buf, "<source dev='%s'/>\n",
> -                                  def->data.ethernet.dev);
> +        virBufferEscapeString(buf, "<source dev='%s'/>\n",
> +                              def->data.ethernet.dev);
>           if (def->data.ethernet.ipaddr)
>               virBufferAsprintf(buf, "<ip address='%s'/>\n",
>                                 def->data.ethernet.ipaddr);

This looks like it could be changed to virBufferEscapeString and drop 
the test. (or is the ip address being mangled by the escape function?)

> -        if (def->data.ethernet.script)
> -            virBufferEscapeString(buf, "<script path='%s'/>\n",
> -                                  def->data.ethernet.script);
> +        virBufferEscapeString(buf, "<script path='%s'/>\n",
> +                              def->data.ethernet.script);
>           break;
>
>       case VIR_DOMAIN_NET_TYPE_BRIDGE:

> @@ -10563,12 +10553,12 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>       }
>       if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee ||
>           def->mem.swap_hard_limit)
> -        virBufferAsprintf(buf, "</memtune>\n");
> +        virBufferAddLit(buf, "</memtune>\n");
>
>       if (def->mem.hugepage_backed) {
> -        virBufferAddLit(buf, "<memoryBacking>\n");
> -        virBufferAddLit(buf, "<hugepages/>\n");
> -        virBufferAddLit(buf, "</memoryBacking>\n");

I'd probably break the first string on a new line too, to have them 
nicely lined up.

> +        virBufferStrcat(buf, "<memoryBacking>\n",
> +                        "<hugepages/>\n",
> +                        "</memoryBacking>\n", NULL);
>       }
>
>       for (n = 0 ; n<  def->cpumasklen ; n++)
> @@ -10626,27 +10616,25 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>           def->cputune.period || def->cputune.quota)
>           virBufferAddLit(buf, "</cputune>\n");
>
> -    if (def->numatune.memory.nodemask)
> -        virBufferAddLit(buf, "<numatune>\n");
> -
>       if (def->numatune.memory.nodemask) {
>           char *nodemask = NULL;
> +
> +        virBufferAddLit(buf, "<numatune>\n");
>           nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask,
>                                            VIR_DOMAIN_CPUMASK_LEN);
>           if (nodemask == NULL) {
> -            virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> -                                 "%s", _("failed to format nodeset for NUMA memory tuning"));
> +            virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                          _("failed to format nodeset for NUMA memory tuning"));
>               goto cleanup;
>           }
>
>           virBufferAsprintf(buf, "<memory mode='%s' nodeset='%s'/>\n",
> -                          virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode),
> +                          virDomainNumatuneMemModeTypeToString(def->numatune.
> +                                                               memory.mode),

They shoul have made terminals with more than 80 cols at the very 
beginning :( This looked a little bit confusing to me at first, 
interpreting the dot as a comma ... but, well, it works.

>                             nodemask);
>           VIR_FREE(nodemask);
> -    }
> -
> -    if (def->numatune.memory.nodemask)
>           virBufferAddLit(buf, "</numatune>\n");
> +    }
>
>       if (def->sysinfo)
>           virDomainSysinfoDefFormat(buf, def->sysinfo);

ACK, you can safely ignore my comments to this patch, as they don't 
address any important issues :). I was looking for 13/13, but couldn't 
find one, so I suppose this is the last one.

Peter