Re: [PATCH qemu v2] QEMU coding style mandates spaces for indentation. This change replaces TABs in block files.

[Kevin Wolf]

Am 30.09.2021 um 14:18 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 9/30/21 12:46, Kevin Wolf wrote:
> > Am 29.09.2021 um 07:30 hat ~farzon geschrieben:
> > > From: Farzon Lotfi <hi@farzon.org>
> > > 
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/371
> > > 
> > > Signed-off-by: Farzon Lotfi <hi@farzon.org>
> > 
> > Just picking one example, but it applies to most hunks of the patch:
> > 
> > > diff --git a/block/parallels.c b/block/parallels.c
> > > index 6ebad2a2bb..629d8aae2b 100644
> > > --- a/block/parallels.c
> > > +++ b/block/parallels.c
> > > @@ -916,11 +916,11 @@ static void parallels_close(BlockDriverState *bs)
> > >   }
> > >   static BlockDriver bdrv_parallels = {
> > > -    .format_name = "parallels",
> > > -    .instance_size       = sizeof(BDRVParallelsState),
> > > -    .bdrv_probe          = parallels_probe,
> > > -    .bdrv_open           = parallels_open,
> > > -    .bdrv_close          = parallels_close,
> > > +    .format_name    = "parallels",
> > > +    .instance_size  = sizeof(BDRVParallelsState),
> > > +    .bdrv_probe     = parallels_probe,
> > > +    .bdrv_open      = parallels_open,
> > > +    .bdrv_close     = parallels_close,
> > >       .bdrv_child_perm          = bdrv_default_perms,
> > >       .bdrv_co_block_status     = parallels_co_block_status,
> > >       .bdrv_has_zero_init       = bdrv_has_zero_init_1,
> > 
> > When we're changing these lines anyway, let's make sure to have
> > consistent alignment with the surrounding code. So I would prefer
> > something like:
> > 
> > +    .bdrv_close               = parallels_close,
> >       .bdrv_child_perm          = bdrv_default_perms,
> > 
> > Rather than:
> > 
> > +    .bdrv_close     = parallels_close,
> >       .bdrv_child_perm          = bdrv_default_perms,
> > 
> > In most cases, there are already inconsistencies in the BlockDriver
> > definitions, but let's use the chance to make it a little better.
> 
> 
> Or may be drop alignment around '=' at all, to have
> 
>    .bdrv_child_perm = bdrv_default_perms,
>    .bdrv_co_block_status = parallels_co_block_status,
>    .bdrv_has_zero_init = bdrv_has_zero_init_1,
> 
> ?

You're right that this would make it easy to keep things consistent, but
I think it hurts readability a lot, even compared to the current, often
inconsistent state.

Kevin