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.