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

Re: [libvirt] [PATCH v2 17/22] Remove unnecessary curly brackets in tools/




On 11/13/2014 09:37 AM, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---
>  tools/virsh-console.c                |  6 ++----
>  tools/virsh-domain.c                 | 27 +++++++++------------------
>  tools/virsh-edit.c                   |  6 ++----
>  tools/virsh-host.c                   |  3 +--
>  tools/virsh-pool.c                   |  6 ++----
>  tools/virsh-volume.c                 | 12 ++++--------
>  tools/virsh.c                        | 18 ++++++------------
>  tools/wireshark/src/packet-libvirt.c |  6 ++----
>  8 files changed, 28 insertions(+), 56 deletions(-)
> 
> diff --git a/tools/virsh-console.c b/tools/virsh-console.c
> index c245df7..f0faf8c 100644
> --- a/tools/virsh-console.c
> +++ b/tools/virsh-console.c
> @@ -224,9 +224,8 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
>                     con->terminalToStream.offset,
>                     avail);
>          if (got < 0) {
> -            if (errno != EAGAIN) {
> +            if (errno != EAGAIN)
>                  virConsoleShutdown(con);
> -            }
>              return;
>          }
>          if (got == 0) {
> @@ -268,9 +267,8 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
>                       con->streamToTerminal.data,
>                       con->streamToTerminal.offset);
>          if (done < 0) {
> -            if (errno != EAGAIN) {
> +            if (errno != EAGAIN)
>                  virConsoleShutdown(con);
> -            }
>              return;
>          }
>          memmove(con->streamToTerminal.data,
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 8e743f1..cde2b4e 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -161,9 +161,8 @@ vshNodeGetCPUCount(virConnectPtr conn)
>      if ((ret = virNodeGetCPUMap(conn, NULL, NULL, 0)) < 0) {
>          /* fall back to nodeinfo */
>          vshResetLibvirtError();
> -        if (virNodeGetInfo(conn, &nodeinfo) == 0) {
> +        if (virNodeGetInfo(conn, &nodeinfo) == 0)
>              ret = VIR_NODEINFO_MAXCPUS(nodeinfo);
> -        }
>      }
>      return ret;
>  }
> @@ -3446,9 +3445,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>          flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
>          snapshots_safe = true;
>      }
> -    if (nvram) {
> +    if (nvram)
>          flags |= VIR_DOMAIN_UNDEFINE_NVRAM;
> -    }
> 
>      if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
>          return false;
> @@ -3498,9 +3496,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>          flags &= ~VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;
>          managed_save_safe = true;
>      }
> -    if (has_snapshots == 0) {
> +    if (has_snapshots == 0)
>          snapshots_safe = true;
> -    }
>      if (has_snapshots_metadata == 0) {
>          flags &= ~VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
>          snapshots_safe = true;
> @@ -6180,9 +6177,8 @@ vshPrintPinInfo(unsigned char *cpumaps, size_t cpumaplen,
>      int cpu, lastcpu;
>      bool bit, lastbit, isInvert;
> 
> -    if (!cpumaps || cpumaplen <= 0 || maxcpu <= 0 || vcpuindex < 0) {
> +    if (!cpumaps || cpumaplen <= 0 || maxcpu <= 0 || vcpuindex < 0)
>          return false;
> -    }
> 
>      bit = lastbit = isInvert = false;
>      lastcpu = -1;
> @@ -6202,9 +6198,8 @@ vshPrintPinInfo(unsigned char *cpumaps, size_t cpumaplen,
>              vshPrint(ctl, "-%d", cpu - 1);
>          lastbit = bit;
>      }
> -    if (bit && !isInvert) {
> +    if (bit && !isInvert)
>          vshPrint(ctl, "-%d", maxcpu - 1);
> -    }
> 
>      return true;
>  }
> @@ -6361,9 +6356,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
>          goto cleanup;
>      }
> 
> -    if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) {
> +    if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0)
>          goto cleanup;
> -    }
> 
>      cpumaplen = VIR_CPU_MAPLEN(maxcpu);
> 
> @@ -7928,13 +7922,11 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
>      kibibytes = VIR_DIV_UP(bytes, 1024);
> 
>      if (flags == -1) {
> -        if (virDomainSetMemory(dom, kibibytes) != 0) {
> +        if (virDomainSetMemory(dom, kibibytes) != 0)
>              ret = false;
> -        }
>      } else {
> -        if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) {
> +        if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0)
>              ret = false;
> -        }
>      }
> 
>      virDomainFree(dom);
> @@ -9488,9 +9480,8 @@ doMigrate(void *opaque)
>      if (vshCommandOptBool(cmd, "rdma-pin-all"))
>          flags |= VIR_MIGRATE_RDMA_PIN_ALL;
> 
> -    if (vshCommandOptBool(cmd, "offline")) {
> +    if (vshCommandOptBool(cmd, "offline"))
>          flags |= VIR_MIGRATE_OFFLINE;
> -    }
> 
>      if (vshCommandOptBool(cmd, "abort-on-error"))
>          flags |= VIR_MIGRATE_ABORT_ON_ERROR;
> diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c
> index 10298f6..41a6d53 100644
> --- a/tools/virsh-edit.c
> +++ b/tools/virsh-edit.c
> @@ -85,9 +85,8 @@ do {
>          goto edit_cleanup;
> 
>      /* Compare original XML with edited.  Has it changed at all? */
> -    if (STREQ(doc, doc_edited)) {
> +    if (STREQ(doc, doc_edited))
>          EDIT_NOT_CHANGED;
> -    }

And this is one where the fears of removing the brackets sets in -
inline MACROs that do not have a 'do { ... } while(0)' definition....

I thought perhaps it'd be vbox and the funky macros there, but if you
look at the #define for EDIT_NOT_CHANGED it differs within the various
virsh-*.c files.

For example, in virsh-domain.c there's:

#define EDIT_NOT_CHANGED \
    vshPrint(ctl, _("Saved image %s XML configuration " \
                    "not changed.\n"), file);           \
    ret = true; goto edit_cleanup;

So, in effect the second line will always be executed as I'm reading things.

there's also :
#define EDIT_NOT_CHANGED                                    \
        vshPrint(ctl, "%s", _("Metadata not changed"));     \
        ret = true;                                         \
        goto edit_cleanup;


and

#define EDIT_NOT_CHANGED \
    vshPrint(ctl, _("Domain %s XML configuration not changed.\n"),  \
             virDomainGetName(dom));                                \
    ret = true; goto edit_cleanup;


I think modifying the macros to have the do {...} while(0); syntax will
do the trick.

John

> 
>   redefine:
>      msg = NULL;
> @@ -109,9 +108,8 @@ do {
>      }
> 
>      /* Everything checks out, so redefine the object. */
> -    if (!msg && !(EDIT_DEFINE)) {
> +    if (!msg && !(EDIT_DEFINE))
>          msg = _("Failed.");
> -    }
> 
>      if (msg) {
>          int c = vshAskReedit(ctl, msg);
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 28b160d..20765e5 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -376,9 +376,8 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
>                  goto cleanup;
> 
>              vshPrint(ctl, _("Node %d:\n"), cell);
> -            for (j = 0; j < npages; j++) {
> +            for (j = 0; j < npages; j++)
>                  vshPrint(ctl, "%uKiB: %lld\n", pagesize[j], counts[j]);
> -            }
>              vshPrint(ctl, "%c", '\n');
>          }
> 
> diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
> index 0b8dba5..a05608b 100644
> --- a/tools/virsh-pool.c
> +++ b/tools/virsh-pool.c
> @@ -470,13 +470,11 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd)
>      if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
>          return false;
> 
> -    if (vshCommandOptBool(cmd, "no-overwrite")) {
> +    if (vshCommandOptBool(cmd, "no-overwrite"))
>          flags |= VIR_STORAGE_POOL_BUILD_NO_OVERWRITE;
> -    }
> 
> -    if (vshCommandOptBool(cmd, "overwrite")) {
> +    if (vshCommandOptBool(cmd, "overwrite"))
>          flags |= VIR_STORAGE_POOL_BUILD_OVERWRITE;
> -    }
> 
>      if (virStoragePoolBuild(pool, flags) == 0) {
>          vshPrint(ctl, _("Pool %s built\n"), name);
> diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
> index 4f810f8..9b6b4c9 100644
> --- a/tools/virsh-volume.c
> +++ b/tools/virsh-volume.c
> @@ -687,9 +687,8 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
>          return false;
>      }
> 
> -    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) {
> +    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name)))
>          return false;
> -    }
> 
>      if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0)
>          goto cleanup;
> @@ -885,9 +884,8 @@ cmdVolDelete(vshControl *ctl, const vshCmd *cmd)
>      bool ret = true;
>      const char *name;
> 
> -    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) {
> +    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name)))
>          return false;
> -    }
> 
>      if (virStorageVolDelete(vol, 0) == 0) {
>          vshPrint(ctl, _("Vol %s deleted\n"), name);
> @@ -945,9 +943,8 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd)
>      int algorithm = VIR_STORAGE_VOL_WIPE_ALG_ZERO;
>      int funcRet;
> 
> -    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) {
> +    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name)))
>          return false;
> -    }
> 
>      if (vshCommandOptStringReq(ctl, cmd, "algorithm", &algorithm_str) < 0)
>          goto out;
> @@ -1741,9 +1738,8 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
>      virStorageVolPtr vol;
>      char * StorageVolPath;
> 
> -    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) {
> +    if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
>          return false;
> -    }
> 
>      if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
>          virStorageVolFree(vol);
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 036b517..eb648bd 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -499,9 +499,8 @@ vshPrintRaw(vshControl *ctl, ...)
>      char *key;
> 
>      va_start(ap, ctl);
> -    while ((key = va_arg(ap, char *)) != NULL) {
> +    while ((key = va_arg(ap, char *)) != NULL)
>          vshPrint(ctl, "%s\r\n", key);
> -    }
>      va_end(ap);
>  }
> 
> @@ -874,9 +873,8 @@ cmdCd(vshControl *ctl, const vshCmd *cmd)
>          return false;
>      }
> 
> -    if (vshCommandOptString(cmd, "dir", &dir) <= 0) {
> +    if (vshCommandOptString(cmd, "dir", &dir) <= 0)
>          dir = dir_malloced = virGetUserDirectory();
> -    }
>      if (!dir)
>          dir = "/";
> 
> @@ -1136,9 +1134,8 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
>      const vshCmdOptDef *ret = NULL;
>      char *alias = NULL;
> 
> -    if (STREQ(name, helpopt.name)) {
> +    if (STREQ(name, helpopt.name))
>          return &helpopt;
> -    }
> 
>      for (i = 0; cmd->opts && cmd->opts[i].name; i++) {
>          const vshCmdOptDef *opt = &cmd->opts[i];
> @@ -1642,9 +1639,8 @@ vshCommandOptString(const vshCmd *cmd, const char *name, const char **value)
>      if (ret <= 0)
>          return ret;
> 
> -    if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK)) {
> +    if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK))
>          return -1;
> -    }
>      *value = arg->data;
>      return 1;
>  }
> @@ -1839,9 +1835,8 @@ vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt)
>      opt = opt ? opt->next : cmd->opts;
> 
>      while (opt) {
> -        if (opt->def->type == VSH_OT_ARGV) {
> +        if (opt->def->type == VSH_OT_ARGV)
>              return opt;
> -        }
>          opt = opt->next;
>      }
>      return NULL;
> @@ -3703,9 +3698,8 @@ main(int argc, char **argv)
>      else
>          progname++;
> 
> -    if ((defaultConn = virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI"))) {
> +    if ((defaultConn = virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")))
>          ctl->name = vshStrdup(ctl, defaultConn);
> -    }
> 
>      vshInitDebug(ctl);
> 
> diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
> index 5c3c369..f7aa7ed 100644
> --- a/tools/wireshark/src/packet-libvirt.c
> +++ b/tools/wireshark/src/packet-libvirt.c
> @@ -249,9 +249,8 @@ find_payload_dissector(guint32 proc, guint32 type,
> 
>      first = pds[0].proc;
>      last = pds[length-1].proc;
> -    if (proc < first || proc > last) {
> +    if (proc < first || proc > last)
>          return NULL;
> -    }
> 
>      pd = &pds[proc-first];
>      /* There is no guarantee to proc numbers has no gap */
> @@ -329,9 +328,8 @@ dissect_libvirt_payload_xdr_data(tvbuff_t *tvb, proto_tree *tree, gint payload_l
>      xdr_destroy(&xdrs);
>      g_free(payload_data);
> 
> -    if (nfds != 0) {
> +    if (nfds != 0)
>          dissect_libvirt_fds(tvb, start + payload_length, nfds);
> -    }
>  }
> 
>  static void
> 


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