Eric Blake wrote:
> 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.
>
Yes, good point. Also, looking at that section of HACKING again, it
says the braces can be omitted "only when that
body occupies a single line".
>
>>
>> /* 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.
>
I removed such changes as you noted above, where there were more than
one line, and pushed the patch.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list