9/30/21 12:46, Kevin Wolf wrote:
Am 29.09.2021 um 07:30 hat ~farzon geschrieben:
From: Farzon Lotfi <h...@farzon.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/371
Signed-off-by: Farzon Lotfi <h...@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,
?
This alignment in definitions is
1. source for alignment inconsistencies
2. infecting logic-changing patches by fixing surround alignment (or having to
add separate patches to adjust old alignments, which is a real waste of time)
3. [1] and [2] will never be helpful for rebase conflicts resolution, when need
to backport/forwardport commits.
and for that all we have a bit more readable definition, which is rarely read as is.
(I think, it's more often used to navigate by tags, like bdrv_open -> jump to
invocation in qcow2.c -> jump to qcow2_open)
--
Best regards,
Vladimir
