> 2022年8月18日 22:56，Mateusz Kusiak <mateusz.kusiak@xxxxxxxxx> 写道：
>
> It prepares update_super0 for change context->update to enum.
> Change if else statements to switch.
>
> Signed-off-by: Mateusz Kusiak <mateusz.kusiak@xxxxxxxxx>
This patch is fine to me almost, except for 2 questions I placed in line.
> ---
> super0.c | 102 ++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 63 insertions(+), 39 deletions(-)
>
> diff --git a/super0.c b/super0.c
> index 37f595ed..4e53f41e 100644
> --- a/super0.c
> +++ b/super0.c
> @@ -502,19 +502,39 @@ static int update_super0(struct supertype *st, struct mdinfo *info,
> int rv = 0;
> int uuid[4];
> mdp_super_t *sb = st->sb;
> + enum update_opt update_enum = map_name(update_options, update);
>
> - if (strcmp(update, "homehost") == 0 &&
> - homehost) {
> - /* note that 'homehost' is special as it is really
> + if (update_enum == UOPT_HOMEHOST && homehost) {
> + /*
> + * note that 'homehost' is special as it is really
> * a "uuid" update.
> */
> uuid_set = 0;
> - update = "uuid";
> + update_enum = UOPT_UUID;
> info->uuid[0] = sb->set_uuid0;
> info->uuid[1] = sb->set_uuid1;
> }
>
> - if (strcmp(update, "sparc2.2")==0 ) {
> + switch (update_enum) {
> + case UOPT_UUID:
> + if (!uuid_set && homehost) {
> + char buf[20];
> + memcpy(info->uuid+2,
> + sha1_buffer(homehost, strlen(homehost), buf),
> + 8);
> + }
> + sb->set_uuid0 = info->uuid[0];
> + sb->set_uuid1 = info->uuid[1];
> + sb->set_uuid2 = info->uuid[2];
> + sb->set_uuid3 = info->uuid[3];
> + if (sb->state & (1<<MD_SB_BITMAP_PRESENT)) {
> + struct bitmap_super_s *bm;
> + bm = (struct bitmap_super_s *)(sb+1);
> + uuid_from_super0(st, uuid);
> + memcpy(bm->uuid, uuid, 16);
> + }
> + break;
> + case UOPT_SPARC22: {
> /* 2.2 sparc put the events in the wrong place
> * So we copy the tail of the superblock
> * up 4 bytes before continuing
> @@ -527,12 +547,15 @@ static int update_super0(struct supertype *st, struct mdinfo *info,
> if (verbose >= 0)
> pr_err("adjusting superblock of %s for 2.2/sparc compatibility.\n",
> devname);
> - } else if (strcmp(update, "super-minor") ==0) {
> + break;
> + }
Wondering there isn't compiler warning for unmatched case/break pair, since this break is inside the {} code block.
Should the ‘break’ be placed after {} pair to match key word ‘case’?
>
[snipped]
> @@ -628,29 +659,15 @@ static int update_super0(struct supertype *st, struct mdinfo *info,
> sb->disks[info->disk.number].minor = info->disk.minor;
> sb->disks[info->disk.number].raid_disk = info->disk.raid_disk;
> sb->disks[info->disk.number].state = info->disk.state;
> - } else if (strcmp(update, "resync") == 0) {
> - /* make sure resync happens */
> + break;
> + case UOPT_RESYNC:
> + /**
> + *make sure resync happens
> + */
The above change doesn’t follow existing code style for comments. How about using the previous one line version?
[snipped]
Thanks.
Coly Li
