[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]