[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH qemu v2] QEMU coding style mandates spaces for indentation. T
From: |
Kevin Wolf |
Subject: |
Re: [PATCH qemu v2] QEMU coding style mandates spaces for indentation. This change replaces TABs in block files. |
Date: |
Thu, 30 Sep 2021 14:55:16 +0200 |
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