Re: [Qemu-devel] [PATCH v2 4/7] block/sheepdog: code beautification

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3008 bytes --]

On 08/30/2017 11:57 AM, Jeff Cody wrote:
> No functional changes, just whitespace manipulation.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/sheepdog.c | 162 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 81 insertions(+), 81 deletions(-)
> 

>  static BlockDriver bdrv_sheepdog = {
> -    .format_name    = "sheepdog",
> -    .protocol_name  = "sheepdog",
> -    .instance_size  = sizeof(BDRVSheepdogState),
> -    .bdrv_parse_filename    = sd_parse_filename,
> -    .bdrv_file_open = sd_open,
> -    .bdrv_reopen_prepare    = sd_reopen_prepare,
> -    .bdrv_reopen_commit     = sd_reopen_commit,
> -    .bdrv_reopen_abort      = sd_reopen_abort,
> -    .bdrv_close     = sd_close,
> -    .bdrv_create    = sd_create,
> -    .bdrv_has_zero_init = bdrv_has_zero_init_1,
> -    .bdrv_getlength = sd_getlength,

The existing style is indeed ugly since it has no consistency,...

> +    .format_name                  = "sheepdog",
> +    .protocol_name                = "sheepdog",
> +    .instance_size                = sizeof(BDRVSheepdogState),
> +    .bdrv_parse_filename          = sd_parse_filename,
> +    .bdrv_file_open               = sd_open,
> +    .bdrv_reopen_prepare          = sd_reopen_prepare,
> +    .bdrv_reopen_commit           = sd_reopen_commit,
> +    .bdrv_reopen_abort            = sd_reopen_abort,
> +    .bdrv_close                   = sd_close,
> +    .bdrv_create                  = sd_create,
> +    .bdrv_has_zero_init           = bdrv_has_zero_init_1,
> +    .bdrv_getlength               = sd_getlength,
>      .bdrv_get_allocated_file_size = sd_get_allocated_file_size,

...but aligning '=' requires mass reformatting if you get any longer
.bdrv_ function callback added down the road.  If it were me, I'd just
consistently use a single space everywhere, as in:

.format_name = "sheepdog",
.protocol_name = "sheepdog",

but that's my personal opinion, and not a hard rule, so I won't be
bothered if you don't take it.

>  static BlockDriver bdrv_sheepdog_tcp = {
> -    .format_name    = "sheepdog",
> -    .protocol_name  = "sheepdog+tcp",
> -    .instance_size  = sizeof(BDRVSheepdogState),
> -    .bdrv_parse_filename    = sd_parse_filename,
> +    .format_name                  = "sheepdog",
> +    .protocol_name                = "sheepdog+tcp",
> +    .instance_size                = sizeof(BDRVSheepdogState),
> +    .bdrv_parse_filename          = sd_parse_filename,
>      .bdrv_file_open = sd_open,

See, with your style, this row now stands out as being missed; with my
style, this line needs no edits.

But your changes are indeed semantic no-ops, so whether you fix the few
unaligned lines per your style, or redo to a consistent style of only a
single space, you can add:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]