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

Re: [libvirt] [PATCH 1/2] nodeinfo: Fix code style and some minor bugs.



On 2012年06月25日 22:43, Peter Krempa wrote:
This patch cleans up some line breaks and fixes two minor bugs:

- buffer pointer increment to the actual length that should be skipped
- jump to cleanup section instead of returning -1
---
  src/nodeinfo.c |   31 +++++++++++++++----------------
  1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index f7d0cc6..97482d9 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -254,19 +254,19 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
              char *p;
              unsigned int ui;

-            buf += 9;
+            buf += 7;
              while (*buf&&  c_isspace(*buf))
                  buf++;

              if (*buf != ':' || !buf[1]) {
-                nodeReportError(VIR_ERR_INTERNAL_ERROR,
-                                "%s", _("parsing cpu MHz from cpuinfo"));
+                nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                _("parsing cpu MHz from cpuinfo"));
                  goto cleanup;
              }

-            if (virStrToLong_ui(buf+1,&p, 10,&ui) == 0
+            if (virStrToLong_ui(buf+1,&p, 10,&ui) == 0&&
                  /* Accept trailing fractional part.  */
-&&  (*p == '\0' || *p == '.' || c_isspace(*p)))
+                (*p == '\0' || *p == '.' || c_isspace(*p)))
                  nodeinfo->mhz = ui;
          }

@@ -279,13 +279,13 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
                  buf++;

              if (*buf != ':' || !buf[1]) {
-                nodeReportError(VIR_ERR_INTERNAL_ERROR,
-                                "%s", _("parsing cpu cores from cpuinfo"));
-                return -1;
+                nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                _("parsing cpu cores from cpuinfo"));
+                goto cleanup;

Instead of chaning "return -1" into "goto cleanup", I think
"goto cleanup" should be changed into "return -1". "sysfs_cpudir"
is NULL anyway before the while loop finished.

              }

-            if (virStrToLong_ui(buf+1,&p, 10,&ui) == 0
-&&  (*p == '\0' || c_isspace(*p)))
+            if (virStrToLong_ui(buf+1,&p, 10,&ui) == 0&&
+                (*p == '\0' || c_isspace(*p)))
                  cpu_cores = ui;
           }
  # elif defined(__powerpc__) || \
@@ -300,14 +300,14 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
                  buf++;

              if (*buf != ':' || !buf[1]) {
-                nodeReportError(VIR_ERR_INTERNAL_ERROR,
-                                "%s", _("parsing cpu MHz from cpuinfo"));
+                nodeReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                _("parsing cpu MHz from cpuinfo"));
                  goto cleanup;
              }

-            if (virStrToLong_ui(buf+1,&p, 10,&ui) == 0
+            if (virStrToLong_ui(buf+1,&p, 10,&ui) == 0&&
                  /* Accept trailing fractional part.  */
-&&  (*p == '\0' || *p == '.' || c_isspace(*p)))
+                (*p == '\0' || *p == '.' || c_isspace(*p)))
                  nodeinfo->mhz = ui;
              /* No other interesting infos are available in /proc/cpuinfo.
               * However, there is a line identifying processor's version,
@@ -327,8 +327,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
          virReportOOMError();
          goto cleanup;
      }
-    cpudir = opendir(sysfs_cpudir);
-    if (cpudir == NULL) {
+    if (!(cpudir = opendir(sysfs_cpudir))) {
          virReportSystemError(errno, _("cannot opendir %s"), sysfs_cpudir);
          goto cleanup;
      }

Others are nice cleanups.

Osier


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