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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list