[libvirt] [PATCH] libxl: remove unnecessary curly braces

As per HACKING, remove some unneeded curly braces in the libxl driver. --- Noticed the unneeded braces while reviewing Chunyan's hostdev passthrough series. Not sure if this qualifies as trivial enough to just push, but best for a quick review anyhow to ensure I didn't botch something. src/libxl/libxl_conf.c | 24 ++++++++---------------- src/libxl/libxl_driver.c | 27 ++++++++++++--------------- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 827dfdd..4362e62 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -244,12 +244,10 @@ libxlMakeCapabilitiesInternal(virArch hostarch, } /* Search for existing matching (model,hvm) tuple */ - for (i = 0; i < nr_guest_archs; i++) { + for (i = 0; i < nr_guest_archs; i++) if ((guest_archs[i].arch == arch) && - guest_archs[i].hvm == hvm) { + guest_archs[i].hvm == hvm) break; - } - } /* Too many arch flavours - highly unlikely ! */ if (i >= ARRAY_CARDINALITY(guest_archs)) @@ -690,10 +688,9 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) if (VIR_ALLOC_N(x_disks, ndisks) < 0) return -1; - for (i = 0; i < ndisks; i++) { + for (i = 0; i < ndisks; i++) if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0) goto error; - } d_config->disks = x_disks; d_config->num_disks = ndisks; @@ -760,10 +757,9 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) if (VIR_ALLOC_N(x_nics, nnics) < 0) return -1; - for (i = 0; i < nnics; i++) { + for (i = 0; i < nnics; i++) if (libxlMakeNic(l_nics[i], &x_nics[i])) goto error; - } d_config->nics = x_nics; d_config->num_nics = nnics; @@ -916,21 +912,17 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0) return -1; - if (libxlMakeDomBuildInfo(vm, d_config) < 0) { + if (libxlMakeDomBuildInfo(vm, d_config) < 0) return -1; - } - if (libxlMakeDiskList(def, d_config) < 0) { + if (libxlMakeDiskList(def, d_config) < 0) return -1; - } - if (libxlMakeNicList(def, d_config) < 0) { + if (libxlMakeNicList(def, d_config) < 0) return -1; - } - if (libxlMakeVfbList(driver, def, d_config) < 0) { + if (libxlMakeVfbList(driver, def, d_config) < 0) return -1; - } d_config->on_reboot = def->onReboot; d_config->on_poweroff = def->onPoweroff; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 9dc7261..dc8b6ba 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -301,13 +301,13 @@ libxlTimeoutRegisterEventHook(void *priv, gettimeofday(&now, NULL); timersub(&abs_t, &now, &res); /* Ensure timeout is not overflowed */ - if (timercmp(&res, &zero, <)) { + if (timercmp(&res, &zero, <)) timeout = 0; - } else if (res.tv_sec > INT_MAX / 1000) { + else if (res.tv_sec > INT_MAX / 1000) timeout = INT_MAX; - } else { + else timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000; - } + info->id = virEventAddTimeout(timeout, libxlTimerCallback, info, libxlEventHookInfoFree); if (info->id < 0) { @@ -888,10 +888,9 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) cpumask = (uint8_t*) def->cputune.vcpupin[vcpu]->cpumask; - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; ++i) { + for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; ++i) if (cpumask[i]) VIR_USE_CPU(cpumap, i); - } map.size = cpumaplen; map.map = cpumap; @@ -1006,10 +1005,10 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virDomainObjAssignDef(vm, def, true, NULL); def = NULL; - if (unlink(managed_save_path) < 0) { + if (unlink(managed_save_path) < 0) VIR_WARN("Failed to remove the managed state %s", managed_save_path); - } + vm->hasManagedSave = false; } VIR_FREE(managed_save_path); @@ -4171,12 +4170,12 @@ libxlNodeGetCellsFreeMemory(virConnectPtr conn, if (lastCell >= nr_nodes) lastCell = nr_nodes - 1; - for (numCells = 0, n = startCell; n <= lastCell; n++) { + for (numCells = 0, n = startCell; n <= lastCell; n++) if (numa_info[n].size == LIBXL_NUMAINFO_INVALID_ENTRY) freeMems[numCells++] = 0; else freeMems[numCells++] = numa_info[n].free; - } + ret = numCells; cleanup: @@ -4447,11 +4446,10 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, VIR_TYPED_PARAM_UINT, sc_info.weight) < 0) goto cleanup; - if (*nparams > 1) { + if (*nparams > 1) if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_SCHEDULER_CAP, VIR_TYPED_PARAM_UINT, sc_info.cap) < 0) goto cleanup; - } if (*nparams > XEN_SCHED_CREDIT_NPARAM) *nparams = XEN_SCHED_CREDIT_NPARAM; @@ -4530,11 +4528,10 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, for (i = 0; i < nparams; ++i) { virTypedParameterPtr param = ¶ms[i]; - if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_WEIGHT)) { + if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_WEIGHT)) sc_info.weight = params[i].value.ui; - } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CAP)) { + else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CAP)) sc_info.cap = params[i].value.ui; - } } if (libxl_domain_sched_params_set(priv->ctx, dom->id, &sc_info) != 0) { -- 1.8.1.4

On 08/14/2013 03:41 PM, Jim Fehlig wrote:
As per HACKING, remove some unneeded curly braces in the libxl driver. ---
Noticed the unneeded braces while reviewing Chunyan's hostdev passthrough series. Not sure if this qualifies as trivial enough to just push, but best for a quick review anyhow to ensure I didn't botch something.
src/libxl/libxl_conf.c | 24 ++++++++---------------- src/libxl/libxl_driver.c | 27 ++++++++++++--------------- 2 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 827dfdd..4362e62 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -244,12 +244,10 @@ libxlMakeCapabilitiesInternal(virArch hostarch, }
/* Search for existing matching (model,hvm) tuple */ - for (i = 0; i < nr_guest_archs; i++) { + for (i = 0; i < nr_guest_archs; i++) if ((guest_archs[i].arch == arch) && - guest_archs[i].hvm == hvm) { + guest_archs[i].hvm == hvm) break; - } - }
The inner brace removal is okay, but I tend to use {} on anything that occupies more than one line (even if only one statement), particularly if more than one of the nested lines have the same indentation (as is the case with the split-line if). That means I think the outer braces should stay.
/* Too many arch flavours - highly unlikely ! */ if (i >= ARRAY_CARDINALITY(guest_archs)) @@ -690,10 +688,9 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) if (VIR_ALLOC_N(x_disks, ndisks) < 0) return -1;
- for (i = 0; i < ndisks; i++) { + for (i = 0; i < ndisks; i++) if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0) goto error; - }
This one's borderline - it's more than one line, but at different indentation, so it's not as visually difficult to spot. I can live with this patch as-is, so weak ACK in case you want to wait for any other opinions. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
On 08/14/2013 03:41 PM, Jim Fehlig wrote:
As per HACKING, remove some unneeded curly braces in the libxl driver. ---
Noticed the unneeded braces while reviewing Chunyan's hostdev passthrough series. Not sure if this qualifies as trivial enough to just push, but best for a quick review anyhow to ensure I didn't botch something.
src/libxl/libxl_conf.c | 24 ++++++++---------------- src/libxl/libxl_driver.c | 27 ++++++++++++--------------- 2 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 827dfdd..4362e62 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -244,12 +244,10 @@ libxlMakeCapabilitiesInternal(virArch hostarch, }
/* Search for existing matching (model,hvm) tuple */ - for (i = 0; i < nr_guest_archs; i++) { + for (i = 0; i < nr_guest_archs; i++) if ((guest_archs[i].arch == arch) && - guest_archs[i].hvm == hvm) { + guest_archs[i].hvm == hvm) break; - } - }
The inner brace removal is okay, but I tend to use {} on anything that occupies more than one line (even if only one statement), particularly if more than one of the nested lines have the same indentation (as is the case with the split-line if). That means I think the outer braces should stay.
Yes, good point. Also, looking at that section of HACKING again, it says the braces can be omitted "only when that body occupies a single line".
/* Too many arch flavours - highly unlikely ! */ if (i >= ARRAY_CARDINALITY(guest_archs)) @@ -690,10 +688,9 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) if (VIR_ALLOC_N(x_disks, ndisks) < 0) return -1;
- for (i = 0; i < ndisks; i++) { + for (i = 0; i < ndisks; i++) if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0) goto error; - }
This one's borderline - it's more than one line, but at different indentation, so it's not as visually difficult to spot.
I can live with this patch as-is, so weak ACK in case you want to wait for any other opinions.
I removed such changes as you noted above, where there were more than one line, and pushed the patch. Regards, Jim
participants (2)
-
Eric Blake
-
Jim Fehlig