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

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



On 08/14/2013 03:41 PM, Jim Fehlig wrote:
> As per HACKING, remove some unneeded curly braces in the
> libxl driver.
> ---
> 
> Noticed the unneeded braces while reviewing Chunyan's hostdev
> passthrough series.  Not sure if this qualifies as trivial
> enough to just push, but best for a quick review anyhow to
> ensure I didn't botch something.
> 
>  src/libxl/libxl_conf.c   | 24 ++++++++----------------
>  src/libxl/libxl_driver.c | 27 ++++++++++++---------------
>  2 files changed, 20 insertions(+), 31 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 827dfdd..4362e62 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -244,12 +244,10 @@ libxlMakeCapabilitiesInternal(virArch hostarch,
>              }
>  
>              /* Search for existing matching (model,hvm) tuple */
> -            for (i = 0; i < nr_guest_archs; i++) {
> +            for (i = 0; i < nr_guest_archs; i++)
>                  if ((guest_archs[i].arch == arch) &&
> -                    guest_archs[i].hvm == hvm) {
> +                    guest_archs[i].hvm == hvm)
>                      break;
> -                }
> -            }

The inner brace removal is okay, but I tend to use {} on anything that
occupies more than one line (even if only one statement), particularly
if more than one of the nested lines have the same indentation (as is
the case with the split-line if).  That means I think the outer braces
should stay.

>  
>              /* Too many arch flavours - highly unlikely ! */
>              if (i >= ARRAY_CARDINALITY(guest_archs))
> @@ -690,10 +688,9 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
>      if (VIR_ALLOC_N(x_disks, ndisks) < 0)
>          return -1;
>  
> -    for (i = 0; i < ndisks; i++) {
> +    for (i = 0; i < ndisks; i++)
>          if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0)
>              goto error;
> -    }

This one's borderline - it's more than one line, but at different
indentation, so it's not as visually difficult to spot.

I can live with this patch as-is, so weak ACK in case you want to wait
for any other opinions.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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