On Wed 18-11-20 09:47:48, Christoph Hellwig wrote:
> The code in devt_from_partuuid is very convoluted. Refactor a bit by
> sanitizing the goto and variable name usage.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
The patch looks good to me. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> init/do_mounts.c | 68 ++++++++++++++++++++++--------------------------
> 1 file changed, 31 insertions(+), 37 deletions(-)
>
> diff --git a/init/do_mounts.c b/init/do_mounts.c
> index aef2f24461c7f1..afa26a4028d25e 100644
> --- a/init/do_mounts.c
> +++ b/init/do_mounts.c
> @@ -105,13 +105,10 @@ static int match_dev_by_uuid(struct device *dev, const void *data)
> */
> static dev_t devt_from_partuuid(const char *uuid_str)
> {
> - dev_t res = 0;
> struct uuidcmp cmp;
> struct device *dev = NULL;
> - struct gendisk *disk;
> - struct hd_struct *part;
> + dev_t devt = 0;
> int offset = 0;
> - bool clear_root_wait = false;
> char *slash;
>
> cmp.uuid = uuid_str;
> @@ -120,52 +117,49 @@ static dev_t devt_from_partuuid(const char *uuid_str)
> /* Check for optional partition number offset attributes. */
> if (slash) {
> char c = 0;
> +
> /* Explicitly fail on poor PARTUUID syntax. */
> - if (sscanf(slash + 1,
> - "PARTNROFF=%d%c", &offset, &c) != 1) {
> - clear_root_wait = true;
> - goto done;
> - }
> + if (sscanf(slash + 1, "PARTNROFF=%d%c", &offset, &c) != 1)
> + goto clear_root_wait;
> cmp.len = slash - uuid_str;
> } else {
> cmp.len = strlen(uuid_str);
> }
>
> - if (!cmp.len) {
> - clear_root_wait = true;
> - goto done;
> - }
> + if (!cmp.len)
> + goto clear_root_wait;
>
> - dev = class_find_device(&block_class, NULL, &cmp,
> - &match_dev_by_uuid);
> + dev = class_find_device(&block_class, NULL, &cmp, &match_dev_by_uuid);
> if (!dev)
> - goto done;
> -
> - res = dev->devt;
> + return 0;
>
> - /* Attempt to find the partition by offset. */
> - if (!offset)
> - goto no_offset;
> + if (offset) {
> + /*
> + * Attempt to find the requested partition by adding an offset
> + * to the partition number found by UUID.
> + */
> + struct hd_struct *part;
>
> - res = 0;
> - disk = part_to_disk(dev_to_part(dev));
> - part = disk_get_part(disk, dev_to_part(dev)->partno + offset);
> - if (part) {
> - res = part_devt(part);
> - put_device(part_to_dev(part));
> + part = disk_get_part(dev_to_disk(dev),
> + dev_to_part(dev)->partno + offset);
> + if (part) {
> + devt = part_devt(part);
> + put_device(part_to_dev(part));
> + }
> + } else {
> + devt = dev->devt;
> }
>
> -no_offset:
> put_device(dev);
> -done:
> - if (clear_root_wait) {
> - pr_err("VFS: PARTUUID= is invalid.\n"
> - "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");
> - if (root_wait)
> - pr_err("Disabling rootwait; root= is invalid.\n");
> - root_wait = 0;
> - }
> - return res;
> + return devt;
> +
> +clear_root_wait:
> + pr_err("VFS: PARTUUID= is invalid.\n"
> + "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");
> + if (root_wait)
> + pr_err("Disabling rootwait; root= is invalid.\n");
> + root_wait = 0;
> + return 0;
> }
>
> /**
> --
> 2.29.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
