| Message ID | 20201118084800.2339180-9-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/20] blk-cgroup: fix a hd_struct leak in blkcg_fill_root_iostats | expand |
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@lst.de> The patch looks good to me. You can add: Reviewed-by: Jan Kara <jack@suse.cz> 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 >
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; } /**
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@lst.de> --- init/do_mounts.c | 68 ++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 37 deletions(-)