[libvirt] [PATCH 0/6] syntax check: catch mismatched {} in if-else

After complaining about it in a recent review, I decided to automate things. Eric Blake (6): maint: use consistent if-else braces in conf and friends maint: use consistent if-else braces in qemu maint: use consistent if-else braces in xen and friends maint: use consistent if-else braces in lxc, vbox, phyp maint: use consistent if-else braces in remaining spots maint: enforce previous if-else {} cleanups cfg.mk | 7 ++++ daemon/stream.c | 6 ++-- src/conf/domain_conf.c | 30 ++++++++--------- src/conf/interface_conf.c | 56 ++++++++++++++++--------------- src/conf/node_device_conf.c | 4 +-- src/conf/nwfilter_conf.c | 27 ++++++++++----- src/conf/secret_conf.c | 12 +++---- src/cpu/cpu_x86.c | 18 +++++----- src/esx/esx_vi_types.c | 6 ++-- src/libxl/libxl_conf.c | 12 +++---- src/lxc/lxc_driver.c | 24 ++++++------- src/lxc/lxc_fuse.c | 34 ++++++++++--------- src/network/bridge_driver.c | 8 ++--- src/node_device/node_device_hal.c | 3 +- src/nwfilter/nwfilter_ebiptables_driver.c | 4 +-- src/nwfilter/nwfilter_gentech_driver.c | 5 +-- src/phyp/phyp_driver.c | 45 ++++++++++++++----------- src/qemu/qemu_capabilities.c | 6 ++-- src/qemu/qemu_command.c | 56 +++++++++++++++++-------------- src/qemu/qemu_driver.c | 36 ++++++++++---------- src/qemu/qemu_hotplug.c | 9 ++--- src/qemu/qemu_monitor_text.c | 30 ++++++----------- src/qemu/qemu_process.c | 3 +- src/remote/remote_driver.c | 14 ++++---- src/rpc/virnetsocket.c | 18 +++++----- src/security/virt-aa-helper.c | 14 ++++---- src/storage/storage_backend_disk.c | 4 +-- src/uml/uml_driver.c | 4 +-- src/util/viralloc.c | 6 ++-- src/util/virbuffer.c | 6 ++-- src/util/virdbus.c | 4 +-- src/util/virnetdev.c | 4 +-- src/util/virnetdevvportprofile.c | 3 +- src/util/virpci.c | 10 +++--- src/util/virsocketaddr.c | 8 ++--- src/util/viruri.c | 38 ++++++++++----------- src/vbox/vbox_common.c | 12 +++---- src/vbox/vbox_tmpl.c | 3 +- src/xen/xen_hypervisor.c | 18 +++++----- src/xen/xend_internal.c | 20 +++++------ src/xenapi/xenapi_driver.c | 13 ++++--- src/xenconfig/xen_common.c | 8 ++--- src/xenconfig/xen_sxpr.c | 18 +++++----- tests/cputest.c | 6 ++-- tests/testutils.c | 4 +-- tools/virsh-domain.c | 8 ++--- tools/virsh-host.c | 8 ++--- 47 files changed, 351 insertions(+), 341 deletions(-) -- 1.9.3

I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements. This patch focuses on code shared between multiple drivers. * src/conf/domain_conf.c (virDomainFSDefParseXML) (virSysinfoParseXML, virDomainNetDefParseXML) (virDomainWatchdogDefParseXML) (virDomainRedirFilterUSBDevDefParseXML): Correct use of {}. * src/conf/interface_conf.c (virInterfaceDefParseDhcp) (virInterfaceDefParseIp, virInterfaceVlanDefFormat) (virInterfaceDefParseStartMode, virInterfaceDefParseBondMode) (virInterfaceDefParseBondMiiCarrier) (virInterfaceDefParseBondArpValid): Likewise. * src/conf/node_device_conf.c (virNodeDevCapStorageParseXML): Likewise. * src/conf/nwfilter_conf.c (virNWFilterRuleDetailsParse) (virNWFilterRuleParse, virNWFilterDefParseXML): Likewise. * src/conf/secret_conf.c (secretXMLParseNode): Likewise. * src/cpu/cpu_x86.c (x86Baseline, x86FeatureLoad, x86ModelLoad): Likewise. * src/network/bridge_driver.c (networkKillDaemon) (networkDnsmasqConfContents): Likewise. * src/node_device/node_device_hal.c (dev_refresh): Likewise. * src/nwfilter/nwfilter_gentech_driver.c (virNWFilterInstantiate): Likewise. * src/nwfilter/nwfilter_ebiptables_driver.c (_iptablesCreateRuleInstance): Likewise. * src/storage/storage_backend_disk.c (virStorageBackendDiskBuildPool): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++--------- src/conf/interface_conf.c | 56 ++++++++++++++++--------------- src/conf/node_device_conf.c | 4 +-- src/conf/nwfilter_conf.c | 27 ++++++++++----- src/conf/secret_conf.c | 12 +++---- src/cpu/cpu_x86.c | 18 +++++----- src/network/bridge_driver.c | 8 ++--- src/node_device/node_device_hal.c | 3 +- src/nwfilter/nwfilter_ebiptables_driver.c | 4 +-- src/nwfilter/nwfilter_gentech_driver.c | 5 +-- src/storage/storage_backend_disk.c | 4 +-- 11 files changed, 92 insertions(+), 79 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 53ef694..508de21 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6579,15 +6579,15 @@ virDomainFSDefParseXML(xmlNodePtr node, xmlStrEqual(cur->name, BAD_CAST "source")) { if (def->type == VIR_DOMAIN_FS_TYPE_MOUNT || - def->type == VIR_DOMAIN_FS_TYPE_BIND) + def->type == VIR_DOMAIN_FS_TYPE_BIND) { source = virXMLPropString(cur, "dir"); - else if (def->type == VIR_DOMAIN_FS_TYPE_FILE) + } else if (def->type == VIR_DOMAIN_FS_TYPE_FILE) { source = virXMLPropString(cur, "file"); - else if (def->type == VIR_DOMAIN_FS_TYPE_BLOCK) + } else if (def->type == VIR_DOMAIN_FS_TYPE_BLOCK) { source = virXMLPropString(cur, "dev"); - else if (def->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) + } else if (def->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) { source = virXMLPropString(cur, "name"); - else if (def->type == VIR_DOMAIN_FS_TYPE_RAM) { + } else if (def->type == VIR_DOMAIN_FS_TYPE_RAM) { usage = virXMLPropString(cur, "usage"); units = virXMLPropString(cur, "units"); } @@ -7129,11 +7129,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.vhostuser->data.nix.path = vhostuser_path; vhostuser_path = NULL; - if (STREQ(vhostuser_mode, "server")) + if (STREQ(vhostuser_mode, "server")) { def->data.vhostuser->data.nix.listen = true; - else if (STREQ(vhostuser_mode, "client")) + } else if (STREQ(vhostuser_mode, "client")) { def->data.vhostuser->data.nix.listen = false; - else { + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Wrong <source> 'mode' attribute " "specified with <interface " @@ -9315,9 +9315,9 @@ virDomainWatchdogDefParseXML(xmlNodePtr node, } action = virXMLPropString(node, "action"); - if (action == NULL) + if (action == NULL) { def->action = VIR_DOMAIN_WATCHDOG_ACTION_RESET; - else { + } else { def->action = virDomainWatchdogActionTypeFromString(action); if (def->action < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -9612,9 +9612,9 @@ virSysinfoParseXML(xmlNodePtr node, "%s", _("malformed <sysinfo> uuid element")); goto error; } - if (uuid_generated) + if (uuid_generated) { memcpy(domUUID, uuidbuf, VIR_UUID_BUFLEN); - else if (memcmp(domUUID, uuidbuf, VIR_UUID_BUFLEN) != 0) { + } else if (memcmp(domUUID, uuidbuf, VIR_UUID_BUFLEN) != 0) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("UUID mismatch between <uuid> and " "<sysinfo>")); @@ -10148,11 +10148,11 @@ virDomainRedirFilterUSBDevDefParseXML(xmlNodePtr node) allow = virXMLPropString(node, "allow"); if (allow) { - if (STREQ(allow, "yes")) + if (STREQ(allow, "yes")) { def->allow = true; - else if (STREQ(allow, "no")) + } else if (STREQ(allow, "no")) { def->allow = false; - else { + } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid allow value, either 'yes' or 'no'")); goto error; diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 103e878..effe5ad 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -154,15 +154,15 @@ virInterfaceDefParseStartMode(virInterfaceDefPtr def, char *tmp; tmp = virXPathString("string(./start/@mode)", ctxt); - if (tmp == NULL) + if (tmp == NULL) { def->startmode = VIR_INTERFACE_START_UNSPECIFIED; - else if (STREQ(tmp, "onboot")) + } else if (STREQ(tmp, "onboot")) { def->startmode = VIR_INTERFACE_START_ONBOOT; - else if (STREQ(tmp, "hotplug")) + } else if (STREQ(tmp, "hotplug")) { def->startmode = VIR_INTERFACE_START_HOTPLUG; - else if (STREQ(tmp, "none")) + } else if (STREQ(tmp, "none")) { def->startmode = VIR_INTERFACE_START_NONE; - else { + } else { virReportError(VIR_ERR_XML_ERROR, _("unknown interface startmode %s"), tmp); VIR_FREE(tmp); @@ -181,21 +181,21 @@ virInterfaceDefParseBondMode(xmlXPathContextPtr ctxt) tmp = virXPathString("string(./@mode)", ctxt); if (tmp == NULL) return VIR_INTERFACE_BOND_NONE; - if (STREQ(tmp, "balance-rr")) + if (STREQ(tmp, "balance-rr")) { ret = VIR_INTERFACE_BOND_BALRR; - else if (STREQ(tmp, "active-backup")) + } else if (STREQ(tmp, "active-backup")) { ret = VIR_INTERFACE_BOND_ABACKUP; - else if (STREQ(tmp, "balance-xor")) + } else if (STREQ(tmp, "balance-xor")) { ret = VIR_INTERFACE_BOND_BALXOR; - else if (STREQ(tmp, "broadcast")) + } else if (STREQ(tmp, "broadcast")) { ret = VIR_INTERFACE_BOND_BCAST; - else if (STREQ(tmp, "802.3ad")) + } else if (STREQ(tmp, "802.3ad")) { ret = VIR_INTERFACE_BOND_8023AD; - else if (STREQ(tmp, "balance-tlb")) + } else if (STREQ(tmp, "balance-tlb")) { ret = VIR_INTERFACE_BOND_BALTLB; - else if (STREQ(tmp, "balance-alb")) + } else if (STREQ(tmp, "balance-alb")) { ret = VIR_INTERFACE_BOND_BALALB; - else { + } else { virReportError(VIR_ERR_XML_ERROR, _("unknown bonding mode %s"), tmp); ret = -1; @@ -213,11 +213,11 @@ virInterfaceDefParseBondMiiCarrier(xmlXPathContextPtr ctxt) tmp = virXPathString("string(./miimon/@carrier)", ctxt); if (tmp == NULL) return VIR_INTERFACE_BOND_MII_NONE; - if (STREQ(tmp, "ioctl")) + if (STREQ(tmp, "ioctl")) { ret = VIR_INTERFACE_BOND_MII_IOCTL; - else if (STREQ(tmp, "netif")) + } else if (STREQ(tmp, "netif")) { ret = VIR_INTERFACE_BOND_MII_NETIF; - else { + } else { virReportError(VIR_ERR_XML_ERROR, _("unknown mii bonding carrier %s"), tmp); ret = -1; @@ -235,13 +235,13 @@ virInterfaceDefParseBondArpValid(xmlXPathContextPtr ctxt) tmp = virXPathString("string(./arpmon/@validate)", ctxt); if (tmp == NULL) return VIR_INTERFACE_BOND_ARP_NONE; - if (STREQ(tmp, "active")) + if (STREQ(tmp, "active")) { ret = VIR_INTERFACE_BOND_ARP_ACTIVE; - else if (STREQ(tmp, "backup")) + } else if (STREQ(tmp, "backup")) { ret = VIR_INTERFACE_BOND_ARP_BACKUP; - else if (STREQ(tmp, "all")) + } else if (STREQ(tmp, "all")) { ret = VIR_INTERFACE_BOND_ARP_ALL; - else { + } else { virReportError(VIR_ERR_XML_ERROR, _("unknown arp bonding validate %s"), tmp); ret = -1; @@ -264,18 +264,19 @@ virInterfaceDefParseDhcp(virInterfaceProtocolDefPtr def, /* Not much to do in the current version */ tmp = virXPathString("string(./@peerdns)", ctxt); if (tmp) { - if (STREQ(tmp, "yes")) + if (STREQ(tmp, "yes")) { def->peerdns = 1; - else if (STREQ(tmp, "no")) + } else if (STREQ(tmp, "no")) { def->peerdns = 0; - else { + } else { virReportError(VIR_ERR_XML_ERROR, _("unknown dhcp peerdns value %s"), tmp); ret = -1; } VIR_FREE(tmp); - } else + } else { def->peerdns = -1; + } ctxt->node = save; return ret; @@ -293,9 +294,9 @@ virInterfaceDefParseIp(virInterfaceIpDefPtr def, def->address = tmp; if (tmp != NULL) { ret = virXPathLong("string(./@prefix)", ctxt, &l); - if (ret == 0) + if (ret == 0) { def->prefix = (int) l; - else if (ret == -2) { + } else if (ret == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid ip address prefix value")); return -1; @@ -961,8 +962,9 @@ virInterfaceVlanDefFormat(virBufferPtr buf, const virInterfaceDef *def) def->data.vlan.devname); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</vlan>\n"); - } else + } else { virBufferAddLit(buf, "/>\n"); + } return 0; } diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 54290ae..2a947df 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -683,9 +683,9 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, goto out; } - if (STREQ(type, "hotpluggable")) + if (STREQ(type, "hotpluggable")) { data->storage.flags |= VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE; - else if (STREQ(type, "removable")) { + } else if (STREQ(type, "removable")) { xmlNodePtr orignode2; data->storage.flags |= VIR_NODE_DEV_CAP_STORAGE_REMOVABLE; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 52f24e4..0a0265d 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -1858,10 +1858,12 @@ virNWFilterRuleDetailsParse(xmlNodePtr node, item->u.u8 = uint_val; found = true; data.ui = uint_val; - } else + } else { rc = -1; - } else + } + } else { rc = -1; + } break; case DATATYPE_UINT16_HEX: @@ -1873,10 +1875,12 @@ virNWFilterRuleDetailsParse(xmlNodePtr node, item->u.u16 = uint_val; found = true; data.ui = uint_val; - } else + } else { rc = -1; - } else + } + } else { rc = -1; + } break; case DATATYPE_UINT32_HEX: @@ -1887,8 +1891,9 @@ virNWFilterRuleDetailsParse(xmlNodePtr node, item->u.u32 = uint_val; found = true; data.ui = uint_val; - } else + } else { rc = -1; + } break; case DATATYPE_IPADDR: @@ -1904,8 +1909,9 @@ virNWFilterRuleDetailsParse(xmlNodePtr node, item->u.u8 = (uint8_t)uint_val; found = true; data.ui = uint_val; - } else + } else { rc = -1; + } } else { if (virSocketAddrParseIPv4(&ipaddr, prop) < 0) { rc = -1; @@ -1951,8 +1957,9 @@ virNWFilterRuleDetailsParse(xmlNodePtr node, item->u.u8 = (uint8_t)uint_val; found = true; data.ui = uint_val; - } else + } else { rc = -1; + } } else { if (virSocketAddrParseIPv6(&ipaddr, prop) < 0) { rc = -1; @@ -2457,8 +2464,9 @@ virNWFilterRuleParse(xmlNodePtr node) i++; if (!virAttr[i].id) break; - } else + } else { break; + } } } @@ -2667,8 +2675,9 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) virNWFilterEntryFree(entry); goto cleanup; } - } else + } else { virNWFilterEntryFree(entry); + } } curr = curr->next; } diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index f958240..4eebae5 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -159,11 +159,11 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) prop = virXPathString("string(./@ephemeral)", ctxt); if (prop != NULL) { - if (STREQ(prop, "yes")) + if (STREQ(prop, "yes")) { def->ephemeral = true; - else if (STREQ(prop, "no")) + } else if (STREQ(prop, "no")) { def->ephemeral = false; - else { + } else { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'ephemeral'")); goto cleanup; @@ -173,11 +173,11 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) prop = virXPathString("string(./@private)", ctxt); if (prop != NULL) { - if (STREQ(prop, "yes")) + if (STREQ(prop, "yes")) { def->private = true; - else if (STREQ(prop, "no")) + } else if (STREQ(prop, "no")) { def->private = false; - else { + } else { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'private'")); goto cleanup; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index b460e8d..af2e08e 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1,7 +1,7 @@ /* * cpu_x86.c: CPU driver for CPUs with x86 compatible CPUID instruction * - * Copyright (C) 2009-2011, 2013 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -710,9 +710,9 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, goto error; } - if (map->features == NULL) + if (map->features == NULL) { map->features = feature; - else { + } else { feature->next = map->features; map->features = feature; } @@ -1048,9 +1048,9 @@ x86ModelLoad(xmlXPathContextPtr ctxt, goto error; } - if (map->models == NULL) + if (map->models == NULL) { map->models = model; - else { + } else { model->next = map->models; map->models = model; } @@ -1882,9 +1882,9 @@ x86Baseline(virCPUDefPtr *cpus, cpu->type = VIR_CPU_TYPE_GUEST; cpu->match = VIR_CPU_MATCH_EXACT; - if (!cpus[0]->vendor) + if (!cpus[0]->vendor) { outputVendor = false; - else if (!(vendor = x86VendorFind(map, cpus[0]->vendor))) { + } else if (!(vendor = x86VendorFind(map, cpus[0]->vendor))) { virReportError(VIR_ERR_OPERATION_FAILED, _("Unknown CPU vendor %s"), cpus[0]->vendor); goto error; @@ -1914,9 +1914,9 @@ x86Baseline(virCPUDefPtr *cpus, goto error; } - if (cpus[i]->vendor) + if (cpus[i]->vendor) { vn = cpus[i]->vendor; - else { + } else { outputVendor = false; if (model->vendor) vn = model->vendor->name; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4b3f07f..2886866 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -782,9 +782,9 @@ networkKillDaemon(pid_t pid, const char *daemonName, const char *networkName) */ for (i = 0; i < 25; i++) { int signum = 0; - if (i == 0) + if (i == 0) { signum = SIGTERM; - else if (i == 15) { + } else if (i == 15) { signum = SIGKILL; signame = "KILL"; } @@ -1221,9 +1221,9 @@ networkDnsmasqConfContents(virNetworkObjPtr network, /* Are we doing RA instead of radvd? */ if (DNSMASQ_RA_SUPPORT(caps)) { - if (ipv6def) + if (ipv6def) { virBufferAddLit(&configbuf, "enable-ra\n"); - else { + } else { for (i = 0; (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, i)); i++) { diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index cd7d399..ec37223 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -546,8 +546,9 @@ dev_refresh(const char *udi) * to sub-capabilities (like net.80203) is nasty ... so avoid it. */ virNodeDeviceObjRemove(&driverState->devs, dev); - } else + } else { VIR_DEBUG("no device named %s", name); + } nodeDeviceUnlock(driverState); if (dev) { diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index d41133c..2b89439 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -1509,9 +1509,9 @@ _iptablesCreateRuleInstance(virFirewallPtr fw, return 0; } - if (rule->action == VIR_NWFILTER_RULE_ACTION_ACCEPT) + if (rule->action == VIR_NWFILTER_RULE_ACTION_ACCEPT) { target = accept_target; - else { + } else { target = virNWFilterJumpTargetTypeToString(rule->action); skipMatch = defMatch; } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 6c7f77b..79fc422 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -1,7 +1,7 @@ /* * nwfilter_gentech_driver.c: generic technology driver * - * Copyright (C) 2011, 2013 Red Hat, Inc. + * Copyright (C) 2011-2014 Red Hat, Inc. * Copyright (C) 2010 IBM Corp. * Copyright (C) 2010 Stefan Berger * @@ -699,8 +699,9 @@ virNWFilterInstantiate(const unsigned char *vmuuid ATTRIBUTE_UNUSED, filter->name, learning); goto err_exit; } - } else + } else { goto err_unresolvable_vars; + } } else if (virHashSize(missing_vars->hashTable) > 1) { goto err_unresolvable_vars; } else if (!forceWithPendingReq && diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index d85f13f..cb6a8d5 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -414,9 +414,9 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; } - if (flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) + if (flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) { ok_to_mklabel = true; - else { + } else { int check; check = virStorageBackendDiskFindLabel( -- 1.9.3

On 09/03/14 23:25, Eric Blake wrote:
I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements.
This patch focuses on code shared between multiple drivers.
* src/conf/domain_conf.c (virDomainFSDefParseXML) (virSysinfoParseXML, virDomainNetDefParseXML) (virDomainWatchdogDefParseXML) (virDomainRedirFilterUSBDevDefParseXML): Correct use of {}. * src/conf/interface_conf.c (virInterfaceDefParseDhcp) (virInterfaceDefParseIp, virInterfaceVlanDefFormat) (virInterfaceDefParseStartMode, virInterfaceDefParseBondMode) (virInterfaceDefParseBondMiiCarrier) (virInterfaceDefParseBondArpValid): Likewise. * src/conf/node_device_conf.c (virNodeDevCapStorageParseXML): Likewise. * src/conf/nwfilter_conf.c (virNWFilterRuleDetailsParse) (virNWFilterRuleParse, virNWFilterDefParseXML): Likewise. * src/conf/secret_conf.c (secretXMLParseNode): Likewise. * src/cpu/cpu_x86.c (x86Baseline, x86FeatureLoad, x86ModelLoad): Likewise. * src/network/bridge_driver.c (networkKillDaemon) (networkDnsmasqConfContents): Likewise. * src/node_device/node_device_hal.c (dev_refresh): Likewise. * src/nwfilter/nwfilter_gentech_driver.c (virNWFilterInstantiate): Likewise. * src/nwfilter/nwfilter_ebiptables_driver.c (_iptablesCreateRuleInstance): Likewise. * src/storage/storage_backend_disk.c (virStorageBackendDiskBuildPool): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++--------- src/conf/interface_conf.c | 56 ++++++++++++++++--------------- src/conf/node_device_conf.c | 4 +-- src/conf/nwfilter_conf.c | 27 ++++++++++----- src/conf/secret_conf.c | 12 +++---- src/cpu/cpu_x86.c | 18 +++++----- src/network/bridge_driver.c | 8 ++--- src/node_device/node_device_hal.c | 3 +- src/nwfilter/nwfilter_ebiptables_driver.c | 4 +-- src/nwfilter/nwfilter_gentech_driver.c | 5 +-- src/storage/storage_backend_disk.c | 4 +-- 11 files changed, 92 insertions(+), 79 deletions(-)
ACK, Peter

I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements. This commit focuses on the qemu driver. * src/qemu/qemu_command.c (qemuParseISCSIString) (qemuParseCommandLineDisk, qemuParseCommandLine) (qemuBuildSmpArgStr, qemuBuildCommandLine) (qemuParseCommandLineDisk, qemuParseCommandLineSmp): Correct use of {}. * src/qemu/qemu_capabilities.c (virQEMUCapsProbeCPUModels): Likewise. * src/qemu/qemu_driver.c (qemuDomainCoreDumpWithFormat) (qemuDomainRestoreFlags, qemuDomainGetInfo) (qemuDomainMergeBlkioDevice): Likewise. * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise. * src/qemu/qemu_monitor_text.c (qemuMonitorTextCreateSnapshot) (qemuMonitorTextLoadSnapshot, qemuMonitorTextDeleteSnapshot): Likewise. * src/qemu/qemu_process.c (qemuProcessStop): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ++--- src/qemu/qemu_command.c | 56 ++++++++++++++++++++++++-------------------- src/qemu/qemu_driver.c | 36 ++++++++++++++-------------- src/qemu/qemu_hotplug.c | 9 +++---- src/qemu/qemu_monitor_text.c | 30 ++++++++---------------- src/qemu/qemu_process.c | 3 +-- 6 files changed, 67 insertions(+), 73 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c70a1a8..854a9b8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -629,11 +629,11 @@ virQEMUCapsProbeCPUModels(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) virCommandPtr cmd; if (qemuCaps->arch == VIR_ARCH_I686 || - qemuCaps->arch == VIR_ARCH_X86_64) + qemuCaps->arch == VIR_ARCH_X86_64) { parse = virQEMUCapsParseX86Models; - else if (qemuCaps->arch == VIR_ARCH_PPC64) + } else if (qemuCaps->arch == VIR_ARCH_PPC64) { parse = virQEMUCapsParsePPCModels; - else { + } else { VIR_DEBUG("don't know how to parse %s CPU models", virArchToString(qemuCaps->arch)); return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c84c7c3..1ca98fb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2791,9 +2791,9 @@ qemuParseISCSIString(virDomainDiskDefPtr def) if (uri->path && (slash = strchr(uri->path + 1, '/')) != NULL) { - if (slash[1] == '\0') + if (slash[1] == '\0') { *slash = '\0'; - else if (virStrToLong_ui(slash + 1, NULL, 10, &lun) == -1) { + } else if (virStrToLong_ui(slash + 1, NULL, 10, &lun) == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid name '%s' for iSCSI disk"), def->src->path); @@ -6491,8 +6491,7 @@ qemuBuildSmpArgStr(const virDomainDef *def, virBufferAsprintf(&buf, ",sockets=%u", def->cpu->sockets); virBufferAsprintf(&buf, ",cores=%u", def->cpu->cores); virBufferAsprintf(&buf, ",threads=%u", def->cpu->threads); - } - else { + } else { virBufferAsprintf(&buf, ",sockets=%u", def->maxvcpus); virBufferAsprintf(&buf, ",cores=%u", 1); virBufferAsprintf(&buf, ",threads=%u", 1); @@ -7807,10 +7806,10 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); goto error; } - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC) - && (def->clock.timers[i]->tickpolicy - != VIR_DOMAIN_TIMER_TICKPOLICY_DELAY) - && (def->clock.timers[i]->tickpolicy != -1)) { + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC) && + (def->clock.timers[i]->tickpolicy + != VIR_DOMAIN_TIMER_TICKPOLICY_DELAY) && + (def->clock.timers[i]->tickpolicy != -1)) { /* a non-default rtc policy was given, but there is no way to implement it in this version of qemu */ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -9986,8 +9985,9 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (VIR_STRDUP(def->src->path, vdi) < 0) goto error; } - } else + } else { def->src->type = VIR_STORAGE_TYPE_FILE; + } } else { def->src->type = VIR_STORAGE_TYPE_FILE; } @@ -10000,20 +10000,22 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, _("pseries systems do not support ide devices '%s'"), val); goto error; } - } else if (STREQ(values[i], "scsi")) + } else if (STREQ(values[i], "scsi")) { def->bus = VIR_DOMAIN_DISK_BUS_SCSI; - else if (STREQ(values[i], "virtio")) + } else if (STREQ(values[i], "virtio")) { def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; - else if (STREQ(values[i], "xen")) + } else if (STREQ(values[i], "xen")) { def->bus = VIR_DOMAIN_DISK_BUS_XEN; - else if (STREQ(values[i], "sd")) + } else if (STREQ(values[i], "sd")) { def->bus = VIR_DOMAIN_DISK_BUS_SD; + } } else if (STREQ(keywords[i], "media")) { if (STREQ(values[i], "cdrom")) { def->device = VIR_DOMAIN_DISK_DEVICE_CDROM; def->src->readonly = true; - } else if (STREQ(values[i], "floppy")) + } else if (STREQ(values[i], "floppy")) { def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; + } } else if (STREQ(keywords[i], "format")) { if (VIR_STRDUP(def->src->driverName, "qemu") < 0) goto error; @@ -10888,8 +10890,9 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, cpu->sockets = sockets; cpu->cores = cores; cpu->threads = threads; - } else if (sockets || cores || threads) + } else if (sockets || cores || threads) { goto syntax; + } ret = 0; @@ -11215,9 +11218,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, if (!(disk = virDomainDiskDefNew())) goto error; - if (STRPREFIX(val, "/dev/")) + if (STRPREFIX(val, "/dev/")) { disk->src->type = VIR_STORAGE_TYPE_BLOCK; - else if (STRPREFIX(val, "nbd:")) { + } else if (STRPREFIX(val, "nbd:")) { disk->src->type = VIR_STORAGE_TYPE_NETWORK; disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_NBD; } else if (STRPREFIX(val, "rbd:")) { @@ -11231,8 +11234,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, disk->src->type = VIR_STORAGE_TYPE_NETWORK; disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_SHEEPDOG; val += strlen("sheepdog:"); - } else + } else { disk->src->type = VIR_STORAGE_TYPE_FILE; + } if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; if (((def->os.arch == VIR_ARCH_PPC64) && @@ -11367,9 +11371,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, const char *token = NULL; WANT_VALUE(); - if (!strchr(val, ',')) + if (!strchr(val, ',')) { qemuParseCommandLineBootDevs(def, val); - else { + } else { token = val; while (token && *token) { if (STRPREFIX(token, "order=")) { @@ -11683,11 +11687,11 @@ qemuParseCommandLine(virCapsPtr qemuCaps, WANT_VALUE(); val += strlen("PIIX4_PM.disable_s3="); - if (STREQ(val, "0")) + if (STREQ(val, "0")) { def->pm.s3 = VIR_TRISTATE_BOOL_YES; - else if (STREQ(val, "1")) + } else if (STREQ(val, "1")) { def->pm.s3 = VIR_TRISTATE_BOOL_NO; - else { + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("invalid value for disable_s3 parameter: " "'%s'"), val); @@ -11700,11 +11704,11 @@ qemuParseCommandLine(virCapsPtr qemuCaps, WANT_VALUE(); val += strlen("PIIX4_PM.disable_s4="); - if (STREQ(val, "0")) + if (STREQ(val, "0")) { def->pm.s4 = VIR_TRISTATE_BOOL_YES; - else if (STREQ(val, "1")) + } else if (STREQ(val, "1")) { def->pm.s4 = VIR_TRISTATE_BOOL_NO; - else { + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("invalid value for disable_s4 parameter: " "'%s'"), val); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 239a300..5121f85 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2559,9 +2559,9 @@ static int qemuDomainGetInfo(virDomainPtr dom, } else if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) { if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; - if (!virDomainObjIsActive(vm)) + if (!virDomainObjIsActive(vm)) { err = 0; - else { + } else { qemuDomainObjEnterMonitor(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); qemuDomainObjExitMonitor(driver, vm); @@ -3698,9 +3698,9 @@ static int qemuDomainCoreDumpWithFormat(virDomainPtr dom, } } - if (qemuDomainObjEndAsyncJob(driver, vm) == 0) + if (qemuDomainObjEndAsyncJob(driver, vm) == 0) { vm = NULL; - else if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + } else if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { qemuDomainRemoveInactive(driver, vm); vm = NULL; } @@ -5647,9 +5647,9 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (virFileWrapperFdClose(wrapperFd) < 0) VIR_WARN("Failed to close %s", path); - if (!qemuDomainObjEndJob(driver, vm)) + if (!qemuDomainObjEndJob(driver, vm)) { vm = NULL; - else if (ret < 0 && !vm->persistent) { + } else if (ret < 0 && !vm->persistent) { qemuDomainRemoveInactive(driver, vm); vm = NULL; } @@ -7708,17 +7708,17 @@ qemuDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, if (STREQ(src->path, dest->path)) { found = true; - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { dest->weight = src->weight; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { dest->riops = src->riops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { dest->wiops = src->wiops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { dest->rbps = src->rbps; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { dest->wbps = src->wbps; - else { + } else { virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"), type); return -1; @@ -7733,17 +7733,17 @@ qemuDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, return -1; dest = &(*dest_array)[*dest_size - 1]; - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { dest->weight = src->weight; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { dest->riops = src->riops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { dest->wiops = src->wiops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { dest->rbps = src->rbps; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { dest->wbps = src->wbps; - else { + } else { *dest_size = *dest_size - 1; return -1; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a364c52..7bc19cd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -966,12 +966,13 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (virDomainCCWAddressAssign(&net->info, priv->ccwaddrs, !net->info.addr.ccw.assigned) < 0) goto cleanup; - } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-s390 net device cannot be hotplugged.")); - else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - virDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) - goto cleanup; + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && + virDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) { + goto cleanup; + } releaseaddr = true; diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index fc54a11..2bc8261 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2726,17 +2726,14 @@ int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name) virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to take snapshot: %s"), reply); goto cleanup; - } - else if (strstr(reply, "No block device can accept snapshots") != NULL) { + } else if (strstr(reply, "No block device can accept snapshots") != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("this domain does not have a device to take snapshots")); goto cleanup; - } - else if (strstr(reply, "Could not open VM state file") != NULL) { + } else if (strstr(reply, "Could not open VM state file") != NULL) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; - } - else if (strstr(reply, "Error") != NULL + } else if (strstr(reply, "Error") != NULL && strstr(reply, "while writing VM") != NULL) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; @@ -2769,27 +2766,22 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name) virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("this domain does not have a device to load snapshots")); goto cleanup; - } - else if (strstr(reply, "Could not find snapshot") != NULL) { + } else if (strstr(reply, "Could not find snapshot") != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, _("the snapshot '%s' does not exist, and was not loaded"), name); goto cleanup; - } - else if (strstr(reply, "Snapshots not supported on device") != NULL) { + } else if (strstr(reply, "Snapshots not supported on device") != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", reply); goto cleanup; - } - else if (strstr(reply, "Could not open VM state file") != NULL) { + } else if (strstr(reply, "Could not open VM state file") != NULL) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; - } - else if (strstr(reply, "Error") != NULL + } else if (strstr(reply, "Error") != NULL && strstr(reply, "while loading VM state") != NULL) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; - } - else if (strstr(reply, "Error") != NULL + } else if (strstr(reply, "Error") != NULL && strstr(reply, "while activating snapshot on") != NULL) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; @@ -2821,12 +2813,10 @@ int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name) virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("this domain does not have a device to delete snapshots")); goto cleanup; - } - else if (strstr(reply, "Snapshots not supported on device") != NULL) { + } else if (strstr(reply, "Snapshots not supported on device") != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", reply); goto cleanup; - } - else if (strstr(reply, "Error") != NULL + } else if (strstr(reply, "Error") != NULL && strstr(reply, "while deleting snapshot") != NULL) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d9547b2..38ed3fe 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4659,8 +4659,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, if (graphics->data.vnc.autoport) { virPortAllocatorRelease(driver->remotePorts, graphics->data.vnc.port); - } - else if (graphics->data.vnc.portReserved) { + } else if (graphics->data.vnc.portReserved) { virPortAllocatorSetUsed(driver->remotePorts, graphics->data.spice.port, false); -- 1.9.3

On 09/03/14 23:25, Eric Blake wrote:
I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements.
This commit focuses on the qemu driver.
* src/qemu/qemu_command.c (qemuParseISCSIString) (qemuParseCommandLineDisk, qemuParseCommandLine) (qemuBuildSmpArgStr, qemuBuildCommandLine) (qemuParseCommandLineDisk, qemuParseCommandLineSmp): Correct use of {}. * src/qemu/qemu_capabilities.c (virQEMUCapsProbeCPUModels): Likewise. * src/qemu/qemu_driver.c (qemuDomainCoreDumpWithFormat) (qemuDomainRestoreFlags, qemuDomainGetInfo) (qemuDomainMergeBlkioDevice): Likewise. * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise. * src/qemu/qemu_monitor_text.c (qemuMonitorTextCreateSnapshot) (qemuMonitorTextLoadSnapshot, qemuMonitorTextDeleteSnapshot): Likewise. * src/qemu/qemu_process.c (qemuProcessStop): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ++--- src/qemu/qemu_command.c | 56 ++++++++++++++++++++++++-------------------- src/qemu/qemu_driver.c | 36 ++++++++++++++-------------- src/qemu/qemu_hotplug.c | 9 +++---- src/qemu/qemu_monitor_text.c | 30 ++++++++---------------- src/qemu/qemu_process.c | 3 +-- 6 files changed, 67 insertions(+), 73 deletions(-)
ACK Peter

I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements. This patch focuses on code related to xen. * src/libxl/libxl_conf.c (libxlCapsInitGuests) (libxlMakeDomBuildInfo): Correct use of {}. * src/xen/xen_hypervisor.c (virXen_getvcpusinfo) (xenHypervisorMakeCapabilitiesInternal): Likewise. * src/xen/xend_internal.c (xenDaemonOpen) (xenDaemonDomainMigratePerform, xend_detect_config_version) (xenDaemonDetachDeviceFlags, xenDaemonDomainMigratePerform) (xenDaemonDomainBlockPeek): Likewise. * src/xenapi/xenapi_driver.c (xenapiConnectListDomains) (xenapiDomainLookupByUUID, xenapiDomainGetOSType): Likewise. * src/xenconfig/xen_common.c (xenParseCPUFeatures, xenFormatNet): Likewise. * src/xenconfig/xen_sxpr.c (xenParseSxpr, xenFormatSxprNet) (xenFormatSxpr): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/libxl/libxl_conf.c | 12 ++++-------- src/xen/xen_hypervisor.c | 18 +++++++++--------- src/xen/xend_internal.c | 20 ++++++++------------ src/xenapi/xenapi_driver.c | 13 ++++++++----- src/xenconfig/xen_common.c | 8 +++----- src/xenconfig/xen_sxpr.c | 18 +++++++++--------- 6 files changed, 41 insertions(+), 48 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 1dbdd9c..ea8b5c7 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -312,17 +312,14 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) pae = 1; else nonpae = 1; - } - else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { + } else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { arch = VIR_ARCH_X86_64; - } - else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { + } else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { arch = VIR_ARCH_ITANIUM; if (subs[3].rm_so != -1 && STRPREFIX(&token[subs[3].rm_so], "be")) ia64_be = 1; - } - else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { + } else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { arch = VIR_ARCH_PPC64; } else if (STRPREFIX(&token[subs[2].rm_so], "armv7l")) { arch = VIR_ARCH_ARMV7L; @@ -645,8 +642,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, if (def->os.nBootDevs == 0) { bootorder[0] = 'c'; bootorder[1] = '\0'; - } - else { + } else { bootorder[def->os.nBootDevs] = '\0'; } if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 0931234..8570653 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1664,8 +1664,9 @@ virXen_getvcpusinfo(int handle, ipt->state = VIR_VCPU_RUNNING; if (op.u.getvcpuinfo.blocked) ipt->state = VIR_VCPU_BLOCKED; - } else + } else { ipt->state = VIR_VCPU_OFFLINE; + } ipt->cpuTime = op.u.getvcpuinfo.cpu_time; ipt->cpu = op.u.getvcpuinfo.online ? (int)op.u.getvcpuinfo.cpu : -1; @@ -1675,8 +1676,9 @@ virXen_getvcpusinfo(int handle, ipt->state = VIR_VCPU_RUNNING; if (op.u.getvcpuinfod5.blocked) ipt->state = VIR_VCPU_BLOCKED; - } else + } else { ipt->state = VIR_VCPU_OFFLINE; + } ipt->cpuTime = op.u.getvcpuinfod5.cpu_time; ipt->cpu = op.u.getvcpuinfod5.online ? (int)op.u.getvcpuinfod5.cpu : -1; @@ -2375,8 +2377,9 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, subs[1].rm_eo-subs[1].rm_so, sizeof(hvm_type)) == NULL) goto no_memory; - } else if (regexec(&flags_pae_rec, line, 0, NULL, 0) == 0) + } else if (regexec(&flags_pae_rec, line, 0, NULL, 0) == 0) { host_pae = 1; + } } } @@ -2427,17 +2430,14 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, pae = 1; else nonpae = 1; - } - else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { + } else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { arch = VIR_ARCH_X86_64; - } - else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { + } else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { arch = VIR_ARCH_ITANIUM; if (subs[3].rm_so != -1 && STRPREFIX(&token[subs[3].rm_so], "be")) ia64_be = 1; - } - else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { + } else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { arch = VIR_ARCH_PPC64; } else { /* XXX surely no other Xen archs exist. Arrrrrrrrrm */ diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 6d1fec7..d9e76fc 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -883,7 +883,7 @@ xend_detect_config_version(virConnectPtr conn) if (value) { if (virStrToLong_i(value, NULL, 10, &priv->xendConfigVersion) < 0) goto cleanup; - } else { + } else { /* Xen prior to 3.0.3 did not have the xend_config_format field, and is implicitly version 1. */ priv->xendConfigVersion = XEND_CONFIG_VERSION_3_0_2; @@ -1206,8 +1206,7 @@ xenDaemonOpen(virConnectPtr conn, if (xenDaemonOpen_unix(conn, conn->uri->path) < 0 || xend_detect_config_version(conn) == -1) goto failed; - } - else if (STRCASEEQ(conn->uri->scheme, "xen")) { + } else if (STRCASEEQ(conn->uri->scheme, "xen")) { /* * try first to open the unix socket */ @@ -2531,8 +2530,7 @@ xenDaemonDetachDeviceFlags(virConnectPtr conn, ret = xend_op(conn, minidef->name, "op", "device_configure", "config", xendev, "dev", ref, NULL); VIR_FREE(xendev); - } - else { + } else { ret = xend_op(conn, minidef->name, "op", "device_destroy", "type", class, "dev", ref, "force", "0", "rm_cfg", "1", NULL); @@ -2767,8 +2765,7 @@ xenDaemonDomainMigratePerform(virConnectPtr conn, if (uriptr->port) snprintf(port, sizeof(port), "%d", uriptr->port); virURIFree(uriptr); - } - else if ((p = strrchr(uri, ':')) != NULL) { /* "hostname:port" */ + } else if ((p = strrchr(uri, ':')) != NULL) { /* "hostname:port" */ int port_nr, n; if (virStrToLong_i(p+1, NULL, 10, &port_nr) < 0) { @@ -2783,8 +2780,7 @@ xenDaemonDomainMigratePerform(virConnectPtr conn, if (VIR_STRDUP(hostname, uri) < 0) return -1; hostname[n] = '\0'; - } - else { /* "hostname" (or IP address) */ + } else { /* "hostname" (or IP address) */ if (VIR_STRDUP(hostname, uri) < 0) return -1; } @@ -3244,13 +3240,13 @@ xenDaemonDomainBlockPeek(virConnectPtr conn, const char *actual; /* Security check: The path must correspond to a block device. */ - if (minidef->id > 0) + if (minidef->id > 0) { root = sexpr_get(conn, "/xend/domain/%d?detail=1", minidef->id); - else if (minidef->id < 0) + } else if (minidef->id < 0) { root = sexpr_get(conn, "/xend/domain/%s?detail=1", minidef->name); - else { + } else { /* This call always fails for dom0. */ virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domainBlockPeek is not supported for dom0")); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 50331c9..723544a 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -472,8 +472,9 @@ xenapiConnectListDomains(virConnectPtr conn, int *ids, int maxids) if (xen_session_get_this_host(session, &host, session)) { xen_host_get_resident_vms(session, &result, host); xen_host_free(host); - } else + } else { xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, NULL); + } if (result != NULL) { for (i = 0; (i < (result->size)) && (i < maxids); i++) { xen_vm_get_domid(session, &t0, result->contents[i]); @@ -656,12 +657,13 @@ xenapiDomainLookupByUUID(virConnectPtr conn, domP->id = record->domid; } xen_vm_record_free(record); - } - else + } else { xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL); + } xen_vm_free(vm); - } else + } else { xenapiSessionErrorHandler(conn, VIR_ERR_NO_DOMAIN, NULL); + } return domP; } @@ -958,8 +960,9 @@ xenapiDomainGetOSType(virDomainPtr dom) ignore_value(VIR_STRDUP(ostype, STREQ(boot_policy, "BIOS order") ? "hvm" : "xen")); VIR_FREE(boot_policy); - } else + } else { xenapiSessionErrorHandler(dom->conn, VIR_ERR_NO_DOMAIN, NULL); + } cleanup: if (vms) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 9beaf6c..accd25f 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -541,7 +541,7 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) if (xenConfigGetBool(conf, "hpet", &val, -1) < 0) return -1; - else if (val != -1) { + if (val != -1) { virDomainTimerDefPtr timer; if (VIR_ALLOC_N(def->clock.timers, 1) < 0 || @@ -1269,12 +1269,10 @@ xenFormatNet(virConnectPtr conn, if (!hvm) { if (net->model != NULL) virBufferAsprintf(&buf, ",model=%s", net->model); - } - else { + } else { if (net->model != NULL && STREQ(net->model, "netfront")) { virBufferAddLit(&buf, ",type=netfront"); - } - else { + } else { if (net->model != NULL) virBufferAsprintf(&buf, ",model=%s", net->model); diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index ff81c36..6c48e97 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1176,8 +1176,9 @@ xenParseSxpr(const struct sexpr *root, _("unknown lifecycle type %s"), tmp); goto error; } - } else + } else { def->onPoweroff = VIR_DOMAIN_LIFECYCLE_DESTROY; + } tmp = sexpr_node(root, "domain/on_reboot"); if (tmp != NULL) { @@ -1186,8 +1187,9 @@ xenParseSxpr(const struct sexpr *root, _("unknown lifecycle type %s"), tmp); goto error; } - } else + } else { def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART; + } tmp = sexpr_node(root, "domain/on_crash"); if (tmp != NULL) { @@ -1196,8 +1198,9 @@ xenParseSxpr(const struct sexpr *root, _("unknown lifecycle type %s"), tmp); goto error; } - } else + } else { def->onCrash = VIR_DOMAIN_LIFECYCLE_DESTROY; + } if (hvm) { if (sexpr_int(root, "domain/image/hvm/acpi")) @@ -1950,12 +1953,10 @@ xenFormatSxprNet(virConnectPtr conn, if (!hvm) { if (def->model != NULL) virBufferEscapeSexpr(buf, "(model '%s')", def->model); - } - else { + } else { if (def->model != NULL && STREQ(def->model, "netfront")) { virBufferAddLit(buf, "(type netfront)"); - } - else { + } else { if (def->model != NULL) { virBufferEscapeSexpr(buf, "(model '%s')", def->model); } @@ -2393,8 +2394,7 @@ xenFormatSxpr(virConnectPtr conn, } } virBufferAddLit(&buf, "))"); - } - else { + } else { virBufferAddLit(&buf, "(serial "); if (xenFormatSxprChr(def->serials[0], &buf) < 0) goto error; -- 1.9.3

On 09/03/14 23:25, Eric Blake wrote:
I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements.
This patch focuses on code related to xen.
* src/libxl/libxl_conf.c (libxlCapsInitGuests) (libxlMakeDomBuildInfo): Correct use of {}. * src/xen/xen_hypervisor.c (virXen_getvcpusinfo) (xenHypervisorMakeCapabilitiesInternal): Likewise. * src/xen/xend_internal.c (xenDaemonOpen) (xenDaemonDomainMigratePerform, xend_detect_config_version) (xenDaemonDetachDeviceFlags, xenDaemonDomainMigratePerform) (xenDaemonDomainBlockPeek): Likewise. * src/xenapi/xenapi_driver.c (xenapiConnectListDomains) (xenapiDomainLookupByUUID, xenapiDomainGetOSType): Likewise. * src/xenconfig/xen_common.c (xenParseCPUFeatures, xenFormatNet): Likewise. * src/xenconfig/xen_sxpr.c (xenParseSxpr, xenFormatSxprNet) (xenFormatSxpr): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/libxl/libxl_conf.c | 12 ++++-------- src/xen/xen_hypervisor.c | 18 +++++++++--------- src/xen/xend_internal.c | 20 ++++++++------------ src/xenapi/xenapi_driver.c | 13 ++++++++----- src/xenconfig/xen_common.c | 8 +++----- src/xenconfig/xen_sxpr.c | 18 +++++++++--------- 6 files changed, 41 insertions(+), 48 deletions(-)
ACK, Peter

I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements. This patch focuses on drivers that had several issues. * src/lxc/lxc_fuse.c (lxcProcGetattr, lxcProcReadMeminfo): Correct use of {}. * src/lxc/lxc_driver.c (lxcDomainMergeBlkioDevice): Likewise. * src/phyp/phyp_driver.c (phypConnectNumOfDomainsGeneric) (phypUUIDTable_Init, openSSHSession, phypStoragePoolListVolumes) (phypConnectListStoragePools, phypDomainSetVcpusFlags) (phypStorageVolGetXMLDesc, phypStoragePoolGetXMLDesc) (phypConnectListDefinedDomains): Likewise. * src/vbox/vbox_common.c (vboxAttachSound, vboxDumpDisplay) (vboxDomainRevertToSnapshot, vboxDomainSnapshotDelete): Likewise. * src/vbox/vbox_tmpl.c (vboxStorageVolGetXMLDesc): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/lxc/lxc_driver.c | 24 ++++++++++++------------ src/lxc/lxc_fuse.c | 34 ++++++++++++++++++---------------- src/phyp/phyp_driver.c | 45 ++++++++++++++++++++++++++------------------- src/vbox/vbox_common.c | 12 ++++++------ src/vbox/vbox_tmpl.c | 3 ++- 5 files changed, 64 insertions(+), 54 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 66d708a..f93360f 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2246,17 +2246,17 @@ lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, if (STREQ(src->path, dest->path)) { found = true; - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { dest->weight = src->weight; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { dest->riops = src->riops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { dest->wiops = src->wiops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { dest->rbps = src->rbps; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { dest->wbps = src->wbps; - else { + } else { virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"), type); return -1; @@ -2272,17 +2272,17 @@ lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, return -1; dest = &(*dest_array)[*dest_size - 1]; - if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { dest->weight = src->weight; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { dest->riops = src->riops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { dest->wiops = src->wiops; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { dest->rbps = src->rbps; - else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { dest->wbps = src->wbps; - else { + } else { *dest_size = *dest_size - 1; return -1; } diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index a3a1275..5c18cff 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -1,4 +1,5 @@ /* + * Copyright (C) 2014 Red Hat, Inc. * Copyright (C) 2012 Fujitsu Limited. * * lxc_fuse.c: fuse filesystem support for libvirt lxc @@ -75,8 +76,9 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf) stbuf->st_atime = sb.st_atime; stbuf->st_ctime = sb.st_ctime; stbuf->st_mtime = sb.st_mtime; - } else + } else { res = -ENOENT; + } cleanup: VIR_FREE(mempath); @@ -163,47 +165,47 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def, *ptr = '\0'; if (STREQ(line, "MemTotal") && - (def->mem.hard_limit || def->mem.max_balloon)) + (def->mem.hard_limit || def->mem.max_balloon)) { virBufferAsprintf(new_meminfo, "MemTotal: %8llu kB\n", meminfo.memtotal); - else if (STREQ(line, "MemFree") && - (def->mem.hard_limit || def->mem.max_balloon)) + } else if (STREQ(line, "MemFree") && + (def->mem.hard_limit || def->mem.max_balloon)) { virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n", (meminfo.memtotal - meminfo.memusage)); - else if (STREQ(line, "Buffers")) + } else if (STREQ(line, "Buffers")) { virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0); - else if (STREQ(line, "Cached")) + } else if (STREQ(line, "Cached")) { virBufferAsprintf(new_meminfo, "Cached: %8llu kB\n", meminfo.cached); - else if (STREQ(line, "Active")) + } else if (STREQ(line, "Active")) { virBufferAsprintf(new_meminfo, "Active: %8llu kB\n", (meminfo.active_anon + meminfo.active_file)); - else if (STREQ(line, "Inactive")) + } else if (STREQ(line, "Inactive")) { virBufferAsprintf(new_meminfo, "Inactive: %8llu kB\n", (meminfo.inactive_anon + meminfo.inactive_file)); - else if (STREQ(line, "Active(anon)")) + } else if (STREQ(line, "Active(anon)")) { virBufferAsprintf(new_meminfo, "Active(anon): %8llu kB\n", meminfo.active_anon); - else if (STREQ(line, "Inactive(anon)")) + } else if (STREQ(line, "Inactive(anon)")) { virBufferAsprintf(new_meminfo, "Inactive(anon): %8llu kB\n", meminfo.inactive_anon); - else if (STREQ(line, "Active(file)")) + } else if (STREQ(line, "Active(file)")) { virBufferAsprintf(new_meminfo, "Active(file): %8llu kB\n", meminfo.active_file); - else if (STREQ(line, "Inactive(file)")) + } else if (STREQ(line, "Inactive(file)")) { virBufferAsprintf(new_meminfo, "Inactive(file): %8llu kB\n", meminfo.inactive_file); - else if (STREQ(line, "Unevictable")) + } else if (STREQ(line, "Unevictable")) { virBufferAsprintf(new_meminfo, "Unevictable: %8llu kB\n", meminfo.unevictable); - else if (STREQ(line, "SwapTotal") && def->mem.swap_hard_limit) + } else if (STREQ(line, "SwapTotal") && def->mem.swap_hard_limit) { virBufferAsprintf(new_meminfo, "SwapTotal: %8llu kB\n", (meminfo.swaptotal - meminfo.memtotal)); - else if (STREQ(line, "SwapFree") && def->mem.swap_hard_limit) + } else if (STREQ(line, "SwapFree") && def->mem.swap_hard_limit) { virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n", (meminfo.swaptotal - meminfo.memtotal - meminfo.swapusage + meminfo.memusage)); - else { + } else { *ptr = ':'; virBufferAdd(new_meminfo, line, -1); } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ea1981a..25f7f2d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -336,16 +336,17 @@ phypConnectNumOfDomainsGeneric(virConnectPtr conn, unsigned int type) const char *state; virBuffer buf = VIR_BUFFER_INITIALIZER; - if (type == 0) + if (type == 0) { state = "|grep Running"; - else if (type == 1) { + } else if (type == 1) { if (system_type == HMC) { state = "|grep \"Not Activated\""; } else { state = "|grep \"Open Firmware\""; } - } else + } else { state = " "; + } virBufferAddLit(&buf, "lssyscfg -r lpar"); if (system_type == HMC) @@ -787,8 +788,9 @@ phypUUIDTable_Init(virConnectPtr conn) VIR_WARN("Unable to generate UUID for domain %d", ids[i]); } - } else + } else { goto cleanup; + } if (phypUUIDTable_WriteFile(conn) == -1) goto cleanup; @@ -1019,8 +1021,9 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virReportError(VIR_ERR_AUTH_FAILED, "%s", _("Authentication failed")); goto disconnect; - } else + } else { goto exit; + } } else if (rc == LIBSSH2_ERROR_NONE) { goto exit; @@ -2204,9 +2207,9 @@ phypStorageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags) goto cleanup; } - if (vol->name != NULL) + if (vol->name != NULL) { voldef.name = vol->name; - else { + } else { VIR_ERROR(_("Unable to determine storage pool's name.")); goto cleanup; } @@ -2313,9 +2316,9 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the volumes */ - if (exit_status < 0 || ret == NULL) + if (exit_status < 0 || ret == NULL) { goto cleanup; - else { + } else { volumes_list = ret; while (got < nvolumes) { @@ -2327,8 +2330,9 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, goto cleanup; char_ptr++; volumes_list = char_ptr; - } else + } else { break; + } } } @@ -2512,9 +2516,9 @@ phypConnectListStoragePools(virConnectPtr conn, char **const pools, int npools) ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the storage pools */ - if (exit_status < 0 || ret == NULL) + if (exit_status < 0 || ret == NULL) { goto cleanup; - else { + } else { storage_pools = ret; while (got < npools) { @@ -2526,8 +2530,9 @@ phypConnectListStoragePools(virConnectPtr conn, char **const pools, int npools) goto cleanup; char_ptr++; storage_pools = char_ptr; - } else + } else { break; + } } } @@ -2643,9 +2648,9 @@ phypStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) virStoragePoolDef def; memset(&def, 0, sizeof(virStoragePoolDef)); - if (pool->name != NULL) + if (pool->name != NULL) { def.name = pool->name; - else { + } else { VIR_ERROR(_("Unable to determine storage pool's name.")); goto err; } @@ -3134,9 +3139,9 @@ phypConnectListDefinedDomains(virConnectPtr conn, char **const names, int nnames ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the domains */ - if (exit_status < 0 || ret == NULL) + if (exit_status < 0 || ret == NULL) { goto cleanup; - else { + } else { domains = ret; while (got < nnames) { @@ -3148,8 +3153,9 @@ phypConnectListDefinedDomains(virConnectPtr conn, char **const names, int nnames goto cleanup; char_ptr++; domains = char_ptr; - } else + } else { break; + } } } @@ -3617,8 +3623,9 @@ phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } else if (ncpus < nvcpus) { operation = 'a'; amount = nvcpus - ncpus; - } else + } else { return 0; + } virBufferAddLit(&buf, "chhwres -r proc"); if (system_type == HMC) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index b186ea8..eecfff6 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1291,8 +1291,7 @@ vboxAttachSound(virDomainDefPtr def, IMachine *machine) if (def->sounds[0]->model == VIR_DOMAIN_SOUND_MODEL_SB16) { gVBoxAPI.UIAudioAdapter.SetAudioController(audioAdapter, AudioControllerType_SB16); - } else - if (def->sounds[0]->model == VIR_DOMAIN_SOUND_MODEL_AC97) { + } else if (def->sounds[0]->model == VIR_DOMAIN_SOUND_MODEL_AC97) { gVBoxAPI.UIAudioAdapter.SetAudioController(audioAdapter, AudioControllerType_AC97); } @@ -3381,8 +3380,9 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } def->ngraphics++; - } else + } else { virReportOOMError(); + } } VBOX_RELEASE(VRDxServer); } @@ -6510,8 +6510,9 @@ static int vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, ret = vboxDomainCreate(dom); if (!ret) gVBoxAPI.snapshotRestore(dom, machine, prevSnapshot); - } else + } else { ret = 0; + } cleanup: VBOX_RELEASE(prevSnapshot); @@ -7093,8 +7094,7 @@ static int vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot delete metadata of a snapshot with children")); goto cleanup; - } else - if (gVBoxAPI.vboxSnapshotRedefine) { + } else if (gVBoxAPI.vboxSnapshotRedefine) { ret = vboxDomainSnapshotDeleteMetadataOnly(snapshot); } goto cleanup; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index f622ac4..e5d6d66 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -3642,8 +3642,9 @@ static char *vboxStorageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags) #else /* VBOX_API_VERSION >= 4000000 */ def.target.capacity = hddLogicalSize; #endif /* VBOX_API_VERSION >= 4000000 */ - } else + } else { defOk = 0; + } rc = VBOX_MEDIUM_FUNC_ARG1(hardDisk, GetSize, &hddActualSize); if (NS_SUCCEEDED(rc) && defOk) -- 1.9.3

On 09/03/14 23:25, Eric Blake wrote:
I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements.
This patch focuses on drivers that had several issues.
* src/lxc/lxc_fuse.c (lxcProcGetattr, lxcProcReadMeminfo): Correct use of {}. * src/lxc/lxc_driver.c (lxcDomainMergeBlkioDevice): Likewise. * src/phyp/phyp_driver.c (phypConnectNumOfDomainsGeneric) (phypUUIDTable_Init, openSSHSession, phypStoragePoolListVolumes) (phypConnectListStoragePools, phypDomainSetVcpusFlags) (phypStorageVolGetXMLDesc, phypStoragePoolGetXMLDesc) (phypConnectListDefinedDomains): Likewise. * src/vbox/vbox_common.c (vboxAttachSound, vboxDumpDisplay) (vboxDomainRevertToSnapshot, vboxDomainSnapshotDelete): Likewise. * src/vbox/vbox_tmpl.c (vboxStorageVolGetXMLDesc): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/lxc/lxc_driver.c | 24 ++++++++++++------------ src/lxc/lxc_fuse.c | 34 ++++++++++++++++++---------------- src/phyp/phyp_driver.c | 45 ++++++++++++++++++++++++++------------------- src/vbox/vbox_common.c | 12 ++++++------ src/vbox/vbox_tmpl.c | 3 ++- 5 files changed, 64 insertions(+), 54 deletions(-)
ACK, Peter

I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements. This patch focuses on all remaining problems, where there weren't enough issues to warrant splitting it further. * src/remote/remote_driver.c (doRemoteOpen): Correct use of {}. * src/security/virt-aa-helper.c (vah_add_path, valid_path, main): Likewise. * src/rpc/virnetsocket.c (virNetSocketNewConnectLibSSH2): Likewise. * src/esx/esx_vi_types.c (esxVI_Type_FromString): Likewise. * src/uml/uml_driver.c (umlDomainDetachDevice): Likewise. * src/util/viralloc.c (virShrinkN): Likewise. * src/util/virbuffer.c (virBufferURIEncodeString): Likewise. * src/util/virdbus.c (virDBusCall): Likewise. * src/util/virnetdev.c (virNetDevValidateConfig): Likewise. * src/util/virnetdevvportprofile.c (virNetDevVPortProfileGetNthParent): Likewise. * src/util/virpci.c (virPCIDeviceIterDevices) (virPCIDeviceWaitForCleanup) (virPCIDeviceIsBehindSwitchLackingACS): Likewise. * src/util/virsocketaddr.c (virSocketAddrGetNumNetmaskBits): Likewise. * src/util/viruri.c (virURIParseParams): Likewise. * daemon/stream.c (daemonStreamHandleAbort): Likewise. * tests/testutils.c (virtTestResult): Likewise. * tests/cputest.c (cpuTestBaseline): Likewise. * tools/virsh-domain.c (cmdDomPMSuspend): Likewise. * tools/virsh-host.c (cmdNodeSuspend): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- daemon/stream.c | 6 +++--- src/esx/esx_vi_types.c | 6 ++---- src/remote/remote_driver.c | 14 +++++++------- src/rpc/virnetsocket.c | 18 +++++++++--------- src/security/virt-aa-helper.c | 14 +++++++------- src/uml/uml_driver.c | 4 ++-- src/util/viralloc.c | 6 +++--- src/util/virbuffer.c | 6 +++--- src/util/virdbus.c | 4 ++-- src/util/virnetdev.c | 4 ++-- src/util/virnetdevvportprofile.c | 3 ++- src/util/virpci.c | 10 ++++------ src/util/virsocketaddr.c | 8 ++++---- src/util/viruri.c | 38 +++++++++++++++++--------------------- tests/cputest.c | 6 +++--- tests/testutils.c | 4 ++-- tools/virsh-domain.c | 8 ++++---- tools/virsh-host.c | 8 ++++---- 18 files changed, 80 insertions(+), 87 deletions(-) diff --git a/daemon/stream.c b/daemon/stream.c index bac39c5..88bc858 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -1,7 +1,7 @@ /* * stream.c: APIs for managing client streams * - * Copyright (C) 2009, 2011 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -612,10 +612,10 @@ daemonStreamHandleAbort(virNetServerClientPtr client, virStreamEventRemoveCallback(stream->st); virStreamAbort(stream->st); - if (msg->header.status == VIR_NET_ERROR) + if (msg->header.status == VIR_NET_ERROR) { virReportError(VIR_ERR_RPC, "%s", _("stream aborted at client request")); - else { + } else { VIR_WARN("unexpected stream status %d", msg->header.status); virReportError(VIR_ERR_RPC, _("stream aborted with unexpected status %d"), diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index f147e74..4c7dc30 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -1,7 +1,7 @@ /* * esx_vi_types.c: client for the VMware VI API 2.5 to manage ESX hosts * - * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2010, 2014 Red Hat, Inc. * Copyright (C) 2009-2011 Matthias Bolte <matthias.bolte@googlemail.com> * * This library is free software; you can redistribute it and/or @@ -873,9 +873,7 @@ esxVI_Type_FromString(const char *type) #include "esx_vi_types.generated.typefromstring" - else { - return esxVI_Type_Other; - } + return esxVI_Type_Other; } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8bc4baa..915e8e5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -606,9 +606,9 @@ doRemoteOpen(virConnectPtr conn, else transport = trans_unix; } else { - if (STRCASEEQ(transport_str, "tls")) + if (STRCASEEQ(transport_str, "tls")) { transport = trans_tls; - else if (STRCASEEQ(transport_str, "unix")) { + } else if (STRCASEEQ(transport_str, "unix")) { if (conn->uri->server) { virReportError(VIR_ERR_INVALID_ARG, _("using unix socket and remote " @@ -618,15 +618,15 @@ doRemoteOpen(virConnectPtr conn, } else { transport = trans_unix; } - } else if (STRCASEEQ(transport_str, "ssh")) + } else if (STRCASEEQ(transport_str, "ssh")) { transport = trans_ssh; - else if (STRCASEEQ(transport_str, "libssh2")) + } else if (STRCASEEQ(transport_str, "libssh2")) { transport = trans_libssh2; - else if (STRCASEEQ(transport_str, "ext")) + } else if (STRCASEEQ(transport_str, "ext")) { transport = trans_ext; - else if (STRCASEEQ(transport_str, "tcp")) + } else if (STRCASEEQ(transport_str, "tcp")) { transport = trans_tcp; - else { + } else { virReportError(VIR_ERR_INVALID_ARG, "%s", _("remote_open: transport in URL not recognised " "(should be tls|unix|ssh|ext|tcp|libssh2)")); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 9780e17..586a0d7 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -843,13 +843,13 @@ virNetSocketNewConnectLibSSH2(const char *host, if (virNetSSHSessionAuthSetCallback(sess, auth) != 0) goto error; - if (STRCASEEQ("auto", knownHostsVerify)) + if (STRCASEEQ("auto", knownHostsVerify)) { verify = VIR_NET_SSH_HOSTKEY_VERIFY_AUTO_ADD; - else if (STRCASEEQ("ignore", knownHostsVerify)) + } else if (STRCASEEQ("ignore", knownHostsVerify)) { verify = VIR_NET_SSH_HOSTKEY_VERIFY_IGNORE; - else if (STRCASEEQ("normal", knownHostsVerify)) + } else if (STRCASEEQ("normal", knownHostsVerify)) { verify = VIR_NET_SSH_HOSTKEY_VERIFY_NORMAL; - else { + } else { virReportError(VIR_ERR_INVALID_ARG, _("Invalid host key verification method: '%s'"), knownHostsVerify); @@ -873,20 +873,20 @@ virNetSocketNewConnectLibSSH2(const char *host, authMethodNext = authMethodsCopy; while ((authMethod = strsep(&authMethodNext, ","))) { - if (STRCASEEQ(authMethod, "keyboard-interactive")) + if (STRCASEEQ(authMethod, "keyboard-interactive")) { ret = virNetSSHSessionAuthAddKeyboardAuth(sess, username, -1); - else if (STRCASEEQ(authMethod, "password")) + } else if (STRCASEEQ(authMethod, "password")) { ret = virNetSSHSessionAuthAddPasswordAuth(sess, uri, username); - else if (STRCASEEQ(authMethod, "privkey")) + } else if (STRCASEEQ(authMethod, "privkey")) { ret = virNetSSHSessionAuthAddPrivKeyAuth(sess, username, privkey, NULL); - else if (STRCASEEQ(authMethod, "agent")) + } else if (STRCASEEQ(authMethod, "agent")) { ret = virNetSSHSessionAuthAddAgentAuth(sess, username); - else { + } else { virReportError(VIR_ERR_INVALID_ARG, _("Invalid authentication method: '%s'"), authMethod); diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index a0b104c..9c3860b 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -579,9 +579,9 @@ valid_path(const char *path, const bool readonly) if (STRNEQLEN(path, "/", 1)) return 1; - if (!virFileExists(path)) + if (!virFileExists(path)) { vah_warning(_("path does not exist, skipping file type checks")); - else { + } else { if (stat(path, &sb) == -1) return -1; @@ -777,9 +777,9 @@ vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursi vah_error(NULL, 0, _("could not find realpath for disk")); return rc; } - } else - if (VIR_STRDUP_QUIET(tmp, path) < 0) - return rc; + } else if (VIR_STRDUP_QUIET(tmp, path) < 0) { + return rc; + } if (strchr(perms, 'w') != NULL) readonly = false; @@ -1269,9 +1269,9 @@ main(int argc, char **argv) APPARMOR_DIR "/libvirt", ctl->uuid) < 0) vah_error(ctl, 0, _("could not allocate memory")); - if (ctl->cmd == 'a') + if (ctl->cmd == 'a') { rc = parserLoad(ctl->uuid); - else if (ctl->cmd == 'R' || ctl->cmd == 'D') { + } else if (ctl->cmd == 'R' || ctl->cmd == 'D') { rc = parserRemove(ctl->uuid); if (ctl->cmd == 'D') { unlink(include_file); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 7039afc..22fa6db 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2362,9 +2362,9 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) if (dev->type == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) { ret = umlDomainDetachUmlDisk(driver, vm, dev); - else { + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("This type of disk cannot be hot unplugged")); } diff --git a/src/util/viralloc.c b/src/util/viralloc.c index be9f0fe..dc423f5 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -1,7 +1,7 @@ /* * viralloc.c: safer memory allocation * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -368,10 +368,10 @@ int virResizeN(void *ptrptr, */ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) { - if (toremove < *countptr) + if (toremove < *countptr) { ignore_value(virReallocN(ptrptr, size, *countptr -= toremove, false, 0, NULL, NULL, 0)); - else { + } else { virFree(ptrptr); *countptr = 0; } diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 025f0ab..52ffa08 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -1,7 +1,7 @@ /* * virbuffer.c: buffers for libvirt * - * Copyright (C) 2005-2008, 2010-2013 Red Hat, Inc. + * Copyright (C) 2005-2008, 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -585,9 +585,9 @@ virBufferURIEncodeString(virBufferPtr buf, const char *str) return; for (p = str; *p; ++p) { - if (c_isalnum(*p)) + if (c_isalnum(*p)) { buf->content[buf->use++] = *p; - else { + } else { uc = (unsigned char) *p; buf->content[buf->use++] = '%'; buf->content[buf->use++] = hex[uc >> 4]; diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 31251fe..a63338a 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1422,9 +1422,9 @@ virDBusCall(DBusConnection *conn, call, VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS, error ? error : &localerror))) { - if (error) + if (error) { ret = 0; - else { + } else { virReportError(VIR_ERR_DBUS_SERVICE, _("%s: %s"), member, localerror.message ? localerror.message : _("unknown error")); } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index fc9ec1e..cf526ec 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1047,9 +1047,9 @@ int virNetDevValidateConfig(const char *ifname, } if (ifindex != -1) { - if (virNetDevGetIndex(ifname, &idx) < 0) + if (virNetDevGetIndex(ifname, &idx) < 0) { goto cleanup; - else if (idx != ifindex) { + } else if (idx != ifindex) { ret = 0; goto cleanup; } diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 0f43b02..75679b0 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -815,8 +815,9 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int if (tb[IFLA_LINK]) { ifindex = *(int *)RTA_DATA(tb[IFLA_LINK]); ifname = NULL; - } else + } else { end = true; + } i++; } diff --git a/src/util/virpci.c b/src/util/virpci.c index f1d4499..d8e465f 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -492,8 +492,7 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate, virPCIDeviceFree(check); ret = -1; break; - } - else if (rc == 1) { + } else if (rc == 1) { VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name, check->name); *matched = check; ret = 1; @@ -1468,8 +1467,7 @@ virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher) ret = 1; break; } - } - else { + } else { in_matching_device = false; /* expected format: <start>-<end> : <domain>:<bus>:<slot>.<function> */ @@ -2272,9 +2270,9 @@ virPCIDeviceIsBehindSwitchLackingACS(virPCIDevicePtr dev) * into play since devices on the root bus can't P2P without going * through the root IOMMU. */ - if (dev->bus == 0) + if (dev->bus == 0) { return 0; - else { + } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to find parent device for %s"), dev->name); diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 20806e2..7cc4bde 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -693,9 +693,9 @@ int virSocketAddrGetNumNetmaskBits(const virSocketAddr *netmask) j = i << 3; while (j < (8 * 4)) { bit = 1 << (7 - (j & 7)); - if ((tm[j >> 3] & bit)) { + if ((tm[j >> 3] & bit)) c++; - } else + else break; j++; } @@ -727,9 +727,9 @@ int virSocketAddrGetNumNetmaskBits(const virSocketAddr *netmask) j = i << 4; while (j < (16 * 8)) { bit = 1 << (15 - (j & 0xf)); - if ((tm[j >> 4] & bit)) { + if ((tm[j >> 4] & bit)) c++; - } else + else break; j++; } diff --git a/src/util/viruri.c b/src/util/viruri.c index 1bb3e97..69e7649 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -1,7 +1,7 @@ /* * viruri.c: URI parsing wrappers for libxml2 functions * - * Copyright (C) 2012 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -80,32 +80,28 @@ virURIParseParams(virURIPtr uri) eq = strchr(query, '='); if (eq && eq >= end) eq = NULL; - /* Empty section (eg. "&&"). */ - if (end == query) + if (end == query) { + /* Empty section (eg. "&&"). */ goto next; - - /* If there is no '=' character, then we have just "name" - * and consistent with CGI.pm we assume value is "". - */ - else if (!eq) { + } else if (!eq) { + /* If there is no '=' character, then we have just "name" + * and consistent with CGI.pm we assume value is "". + */ name = xmlURIUnescapeString(query, end - query, NULL); if (!name) goto no_memory; - } - /* Or if we have "name=" here (works around annoying - * problem when calling xmlURIUnescapeString with len = 0). - */ - else if (eq+1 == end) { + } else if (eq+1 == end) { + /* Or if we have "name=" here (works around annoying + * problem when calling xmlURIUnescapeString with len = 0). + */ name = xmlURIUnescapeString(query, eq - query, NULL); if (!name) goto no_memory; - } - /* If the '=' character is at the beginning then we have - * "=value" and consistent with CGI.pm we _ignore_ this. - */ - else if (query == eq) + } else if (query == eq) { + /* If the '=' character is at the beginning then we have + * "=value" and consistent with CGI.pm we _ignore_ this. + */ goto next; - - /* Otherwise it's "name=value". */ - else { + } else { + /* Otherwise it's "name=value". */ name = xmlURIUnescapeString(query, eq - query, NULL); if (!name) goto no_memory; diff --git a/tests/cputest.c b/tests/cputest.c index 38cd71e..94c0ebc 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -1,7 +1,7 @@ /* * cputest.c: Test the libvirtd internal CPU APIs * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -333,9 +333,9 @@ cpuTestBaseline(const void *arg) baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags); if (data->result < 0) { virResetLastError(); - if (!baseline) + if (!baseline) { ret = 0; - else if (virTestGetVerbose()) { + } else if (virTestGetVerbose()) { fprintf(stderr, "\n%-70s... ", "cpuBaseline was expected to fail but it succeeded"); } diff --git a/tests/testutils.c b/tests/testutils.c index 5bdfcc5..dd65fe8 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -115,9 +115,9 @@ void virtTestResult(const char *name, int ret, const char *msg, ...) testCounter++; if (virTestGetVerbose()) { fprintf(stderr, "%3zu) %-60s ", testCounter, name); - if (ret == 0) + if (ret == 0) { fprintf(stderr, "OK\n"); - else { + } else { fprintf(stderr, "FAILED\n"); if (msg) { char *str; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c75cd73..68d49d6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2920,13 +2920,13 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) goto cleanup; - if (STREQ(target, "mem")) + if (STREQ(target, "mem")) { suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM; - else if (STREQ(target, "disk")) + } else if (STREQ(target, "disk")) { suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK; - else if (STREQ(target, "hybrid")) + } else if (STREQ(target, "hybrid")) { suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID; - else { + } else { vshError(ctl, "%s", _("Invalid target")); goto cleanup; } diff --git a/tools/virsh-host.c b/tools/virsh-host.c index ad821b3..7fc2120 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -803,13 +803,13 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd) return false; } - if (STREQ(target, "mem")) + if (STREQ(target, "mem")) { suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM; - else if (STREQ(target, "disk")) + } else if (STREQ(target, "disk")) { suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK; - else if (STREQ(target, "hybrid")) + } else if (STREQ(target, "hybrid")) { suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID; - else { + } else { vshError(ctl, "%s", _("Invalid target")); return false; } -- 1.9.3

On 09/03/14 23:25, Eric Blake wrote:
I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements.
This patch focuses on all remaining problems, where there weren't enough issues to warrant splitting it further.
* src/remote/remote_driver.c (doRemoteOpen): Correct use of {}. * src/security/virt-aa-helper.c (vah_add_path, valid_path, main): Likewise. * src/rpc/virnetsocket.c (virNetSocketNewConnectLibSSH2): Likewise. * src/esx/esx_vi_types.c (esxVI_Type_FromString): Likewise. * src/uml/uml_driver.c (umlDomainDetachDevice): Likewise. * src/util/viralloc.c (virShrinkN): Likewise. * src/util/virbuffer.c (virBufferURIEncodeString): Likewise. * src/util/virdbus.c (virDBusCall): Likewise. * src/util/virnetdev.c (virNetDevValidateConfig): Likewise. * src/util/virnetdevvportprofile.c (virNetDevVPortProfileGetNthParent): Likewise. * src/util/virpci.c (virPCIDeviceIterDevices) (virPCIDeviceWaitForCleanup) (virPCIDeviceIsBehindSwitchLackingACS): Likewise. * src/util/virsocketaddr.c (virSocketAddrGetNumNetmaskBits): Likewise. * src/util/viruri.c (virURIParseParams): Likewise. * daemon/stream.c (daemonStreamHandleAbort): Likewise. * tests/testutils.c (virtTestResult): Likewise. * tests/cputest.c (cpuTestBaseline): Likewise. * tools/virsh-domain.c (cmdDomPMSuspend): Likewise. * tools/virsh-host.c (cmdNodeSuspend): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- daemon/stream.c | 6 +++--- src/esx/esx_vi_types.c | 6 ++---- src/remote/remote_driver.c | 14 +++++++------- src/rpc/virnetsocket.c | 18 +++++++++--------- src/security/virt-aa-helper.c | 14 +++++++------- src/uml/uml_driver.c | 4 ++-- src/util/viralloc.c | 6 +++--- src/util/virbuffer.c | 6 +++--- src/util/virdbus.c | 4 ++-- src/util/virnetdev.c | 4 ++-- src/util/virnetdevvportprofile.c | 3 ++- src/util/virpci.c | 10 ++++------ src/util/virsocketaddr.c | 8 ++++---- src/util/viruri.c | 38 +++++++++++++++++--------------------- tests/cputest.c | 6 +++--- tests/testutils.c | 4 ++-- tools/virsh-domain.c | 8 ++++---- tools/virsh-host.c | 8 ++++---- 18 files changed, 80 insertions(+), 87 deletions(-)
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index f147e74..4c7dc30 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c
@@ -873,9 +873,7 @@ esxVI_Type_FromString(const char *type)
#include "esx_vi_types.generated.typefromstring"
This file doesn't adhere to the style. (Generated by esx_vi_generator.py)
- else { - return esxVI_Type_Other; - } + return esxVI_Type_Other; }
...
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 7039afc..22fa6db 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2362,9 +2362,9 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml)
if (dev->type == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) { ret = umlDomainDetachUmlDisk(driver, vm, dev); - else { + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("This type of disk cannot be hot unplugged"));
This is technically a single statement. I presume it's a false positive from your checker.
}
...
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index fc9ec1e..cf526ec 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1047,9 +1047,9 @@ int virNetDevValidateConfig(const char *ifname, }
if (ifindex != -1) { - if (virNetDevGetIndex(ifname, &idx) < 0) + if (virNetDevGetIndex(ifname, &idx) < 0) { goto cleanup;
In the previous patch you've solved one of such situations by dropping the else keyword and letting the next if to be free-standing. Anyways, this is correct.
- else if (idx != ifindex) { + } else if (idx != ifindex) { ret = 0; goto cleanup; }
...
diff --git a/src/util/viruri.c b/src/util/viruri.c index 1bb3e97..69e7649 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c
...
@@ -80,32 +80,28 @@ virURIParseParams(virURIPtr uri) eq = strchr(query, '='); if (eq && eq >= end) eq = NULL;
- /* Empty section (eg. "&&"). */ - if (end == query) + if (end == query) { + /* Empty section (eg. "&&"). */ goto next; - - /* If there is no '=' character, then we have just "name" - * and consistent with CGI.pm we assume value is "". - */ - else if (!eq) { + } else if (!eq) { + /* If there is no '=' character, then we have just "name" + * and consistent with CGI.pm we assume value is "". + */ name = xmlURIUnescapeString(query, end - query, NULL); if (!name) goto no_memory; - } - /* Or if we have "name=" here (works around annoying - * problem when calling xmlURIUnescapeString with len = 0). - */ - else if (eq+1 == end) { + } else if (eq+1 == end) {
Aren't we enforcing spaces around operators?
+ /* Or if we have "name=" here (works around annoying + * problem when calling xmlURIUnescapeString with len = 0). + */ name = xmlURIUnescapeString(query, eq - query, NULL); if (!name) goto no_memory; - } - /* If the '=' character is at the beginning then we have - * "=value" and consistent with CGI.pm we _ignore_ this. - */ - else if (query == eq) + } else if (query == eq) { + /* If the '=' character is at the beginning then we have + * "=value" and consistent with CGI.pm we _ignore_ this. + */ goto next; - - /* Otherwise it's "name=value". */ - else { + } else { + /* Otherwise it's "name=value". */ name = xmlURIUnescapeString(query, eq - query, NULL); if (!name) goto no_memory;
ACK, Peter

On 09/04/2014 06:41 AM, Peter Krempa wrote:
On 09/03/14 23:25, Eric Blake wrote:
I'm about to add a syntax check that enforces our documented HACKING style of always using matching {} on if-else statements.
@@ -873,9 +873,7 @@ esxVI_Type_FromString(const char *type)
#include "esx_vi_types.generated.typefromstring"
This file doesn't adhere to the style. (Generated by esx_vi_generator.py)
- else { - return esxVI_Type_Other; - } + return esxVI_Type_Other; }
Easy enough to fix.
if (dev->type == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) { ret = umlDomainDetachUmlDisk(driver, vm, dev); - else { + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("This type of disk cannot be hot unplugged"));
This is technically a single statement. I presume it's a false positive from your checker.
It was just as easy to fix it in either direction; the checker flagged 'else {' as being unbalanced. I'll drop the else{} instead of adding if{}.
+++ b/src/util/virnetdev.c @@ -1047,9 +1047,9 @@ int virNetDevValidateConfig(const char *ifname, }
if (ifindex != -1) { - if (virNetDevGetIndex(ifname, &idx) < 0) + if (virNetDevGetIndex(ifname, &idx) < 0) { goto cleanup;
In the previous patch you've solved one of such situations by dropping the else keyword and letting the next if to be free-standing. Anyways, this is correct.
Fixed.
- /* Or if we have "name=" here (works around annoying - * problem when calling xmlURIUnescapeString with len = 0). - */ - else if (eq+1 == end) { + } else if (eq+1 == end) {
Aren't we enforcing spaces around operators?
Not consistently :( But not something for this patch to tweak. Maybe I'll have to look into why the syntax checker isn't flagging this, but it would be another day.
ACK,
Here's what I squashed in: diff --git i/src/esx/esx_vi_generator.py w/src/esx/esx_vi_generator.py index 0b75f18..bc026bd 100755 --- i/src/esx/esx_vi_generator.py +++ w/src/esx/esx_vi_generator.py @@ -3,6 +3,7 @@ # # esx_vi_generator.py: generates most of the SOAP type mapping code # +# Copyright (C) 2014 Red Hat, Inc. # Copyright (C) 2010-2012 Matthias Bolte <matthias.bolte@googlemail.com> # Copyright (C) 2013 Ata E Husain Bohra <ata.husain@hotmail.com> # @@ -433,9 +434,8 @@ class Type: def generate_typefromstring(self): - string = " else if (STREQ(type, \"%s\")) {\n" % self.name + string = " if (STREQ(type, \"%s\"))\n" % self.name string += " return esxVI_Type_%s;\n" % self.name - string += " }\n" return string diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c index 22fa6db..5bede07 100644 --- i/src/uml/uml_driver.c +++ w/src/uml/uml_driver.c @@ -2362,12 +2362,11 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) if (dev->type == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) { + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) ret = umlDomainDetachUmlDisk(driver, vm, dev); - } else { + else virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("This type of disk cannot be hot unplugged")); - } } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("This type of device cannot be hot unplugged")); diff --git i/src/util/virnetdev.c w/src/util/virnetdev.c index cf526ec..6a8c6ae 100644 --- i/src/util/virnetdev.c +++ w/src/util/virnetdev.c @@ -1047,9 +1047,9 @@ int virNetDevValidateConfig(const char *ifname, } if (ifindex != -1) { - if (virNetDevGetIndex(ifname, &idx) < 0) { + if (virNetDevGetIndex(ifname, &idx) < 0) goto cleanup; - } else if (idx != ifindex) { + if (idx != ifindex) { ret = 0; goto cleanup; } -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Done as a separate commit in case earlier cleanups are backported independently. * cfg.mk (sc_require_space_before_label): New rule. Signed-off-by: Eric Blake <eblake@redhat.com> --- cfg.mk | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cfg.mk b/cfg.mk index 307009e..122cf58 100644 --- a/cfg.mk +++ b/cfg.mk @@ -910,6 +910,13 @@ sc_require_space_before_label: halt="Top-level labels should be indented by one space" \ $(_sc_search_regexp) +# Doesn't catch all cases of mismatched braces across if-else, but it helps +sc_require_if_else_matching_braces: + @prohibit='( else( if .*\))? {|} else( if .*\))?$$)' \ + in_vc_files='\.[chx]$$' \ + halt="if one side of if-else uses {}, both sides must use it" \ + $(_sc_search_regexp) + sc_curly_braces_style: @files=$$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); \ $(GREP) -nHP \ -- 1.9.3

On 09/03/14 23:25, Eric Blake wrote:
Done as a separate commit in case earlier cleanups are backported independently.
* cfg.mk (sc_require_space_before_label): New rule.
Signed-off-by: Eric Blake <eblake@redhat.com> --- cfg.mk | 7 +++++++ 1 file changed, 7 insertions(+)
Doesn't apply cleanly if your "hanging brace" series patch 2/2 is applied.
diff --git a/cfg.mk b/cfg.mk index 307009e..122cf58 100644 --- a/cfg.mk +++ b/cfg.mk @@ -910,6 +910,13 @@ sc_require_space_before_label: halt="Top-level labels should be indented by one space" \ $(_sc_search_regexp)
+# Doesn't catch all cases of mismatched braces across if-else, but it helps +sc_require_if_else_matching_braces: + @prohibit='( else( if .*\))? {|} else( if .*\))?$$)' \ + in_vc_files='\.[chx]$$' \ + halt="if one side of if-else uses {}, both sides must use it" \ + $(_sc_search_regexp)
Seems to do the right thing, but my regex-fu isn't that strong so I might have missed something.
+ sc_curly_braces_style: @files=$$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); \ $(GREP) -nHP \
ACK Peter

On 09/04/2014 05:26 AM, Peter Krempa wrote:
On 09/03/14 23:25, Eric Blake wrote:
Done as a separate commit in case earlier cleanups are backported independently.
* cfg.mk (sc_require_space_before_label): New rule.
Signed-off-by: Eric Blake <eblake@redhat.com> --- cfg.mk | 7 +++++++ 1 file changed, 7 insertions(+)
Doesn't apply cleanly if your "hanging brace" series patch 2/2 is applied.
Well, I _did_ say in the cover letter to that series that this one goes first :) Just because you chose to review the low-hanging-fruit of the shorter series first...
+# Doesn't catch all cases of mismatched braces across if-else, but it helps +sc_require_if_else_matching_braces: + @prohibit='( else( if .*\))? {|} else( if .*\))?$$)' \ + in_vc_files='\.[chx]$$' \ + halt="if one side of if-else uses {}, both sides must use it" \ + $(_sc_search_regexp)
Seems to do the right thing, but my regex-fu isn't that strong so I might have missed something.
Basically, the cases it doesn't catch are: if (...) { blah; } else if (... && ...) blah; or the counterpart if (...) blah; else if (... && ...) { blah; } where the if condition is broken across multiple lines (it only looks for obvious cases detectable in a single line). I can always submit a followup patch later if I think of a bright way to handle that.
+ sc_curly_braces_style: @files=$$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); \ $(GREP) -nHP \
ACK
Thanks; I'll push the series shortly, after addressing your nits in earlier patches. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Peter Krempa