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