Re: [Qemu-devel] [PATCH 1/7] block/vpc: Fix most coding style warnings and errors

[Stefan Weil]

Am 05.02.2013 11:20, schrieb Kevin Wolf:
> Am 01.02.2013 22:51, schrieb Stefan Weil:
>> Only C99 comments remain unfixed.
>>
>> Signed-off-by: Stefan Weil <address@hidden>
>> ---
>>  block/vpc.c |   94 
>> +++++++++++++++++++++++++++++++++--------------------------
>>  1 file changed, 52 insertions(+), 42 deletions(-)
>> @@ -578,8 +589,8 @@ static int calculate_geometry(int64_t total_sectors, 
>> uint16_t* cyls,
>>  
>>  static int create_dynamic_disk(int fd, uint8_t *buf, int64_t total_sectors)
>>  {
>> -    struct vhd_dyndisk_header* dyndisk_header =
>> -        (struct vhd_dyndisk_header*) buf;
>> +    struct vhd_dyndisk_header *dyndisk_header =
>> +        (struct vhd_dyndisk_header *)buf;
> Please leave the space after the closing bracket.

$ git grep ' \*) [a-z]'|wc
    858    6568   59803
$ git grep ' \*)[a-z]'|wc
   1469    9107  101610

$ git grep ' \*) [a-z]' block|wc
      7      55     499
$ git grep ' \*)[a-z]' block|wc
     52     322    3514

The majority of code does not use a space in pointer type casts.
For block/vpc.c, there were 4 such type casts using a space and
one without.

Here the line was modified because of a CODING_STYLE violation,
and I additionally removed the space to be compatible with
the other code in QEMU and especially in block/*.

If you want the space nevertheless, I'll leave it there.

>>      size_t block_size, num_bat_entries;
>>      int i;
>>      int ret = -EIO;
>> @@ -709,8 +720,7 @@ static int vpc_create(const char *filename, 
>> QEMUOptionParameter *options)
>>      total_sectors = total_size / BDRV_SECTOR_SIZE;
>>      for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl; i++) {
>>          if (calculate_geometry(total_sectors + i, &cyls, &heads,
>> -                               &secs_per_cyl))
>> -        {
>> +                               &secs_per_cyl)) {
>>              ret = -EFBIG;
>>              goto fail;
>>          }
> This is not an improvement. CODING_STYLE says:
>
>   The opening brace is on the line that contains the control
>   flow statement that introduces the new block
>
> It wasn't on the line of the "if" before the patch, and it isn't after
> the patch (and it can't because of the 80 characters/line maximum, which
> is probably the more important rule). I like the readabilty of the
> existing version better.
>
> Kevin


This is what checkpatch.pl says:

ERROR: that open brace { should be on the previous line
#729: FILE: vpc.c:729:
+        if (calculate_geometry(total_sectors + i, &cyls, &heads,
+                               &secs_per_cyl))
+        {

The control flow statement may include more than one line.
Here it has two lines, and CODING_STYLE enforces my change.

I'll send a rebased version of my patch series as soon as
these and any open questions are solved.The rebased patches are
available from git://qemu.weilnetz.de/qemu.git.

Cheers,
Stefan