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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list