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

Re: [libvirt] [PATCH 5/6] maint: use consistent if-else braces in remaining spots



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


Attachment: signature.asc
Description: OpenPGP digital signature


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