* [PATCH v3 0/2] 'part name' subcommand and some robustification
@ 2025-11-10 20:54 Rasmus Villemoes
2025-11-10 20:54 ` [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info Rasmus Villemoes
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Rasmus Villemoes @ 2025-11-10 20:54 UTC (permalink / raw)
To: u-boot; +Cc: Quentin Schulz, Tom Rini, Rasmus Villemoes
Implement a "part name" subcommand, mirroring the existing "part
number" subcommand.
In the discussion for v1 of that, it came up that there's a bit of
inconsistency in how much and what one can assume to be initialized in
'struct disk_partition' after a successful call of one of the
get_info* family of functions. Patch 1/2 tries to consolidate
that by making sure all ->get_info invocations go through a common
helper that at least always initializes the string members.
Quentin, I've taken the liberty of including your Acks, as the
incremental diff in patch 1 is quite minor, but do speak up if I
should not have done that.
v3: Make sure part_get_type_by_name() can only ever return 0, -ENOSYS
(in case ->get_info is not implemented) and -ENOENT, rather than
anything that ->get_info might return.
v2: https://lore.kernel.org/u-boot/20251020121100.1742812-1-ravi@prevas.dk/
Rasmus Villemoes (2):
disk/part.c: ensure strings in struct disk_partition are valid after
successful get_info
cmd/part.c: implement "part name" subcommand
cmd/gpt.c | 4 +--
cmd/part.c | 16 ++++++++++-
disk/part.c | 63 +++++++++++++++++++++++++-----------------
doc/usage/cmd/part.rst | 13 +++++++++
include/part.h | 16 +++++++++++
5 files changed, 83 insertions(+), 29 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info 2025-11-10 20:54 [PATCH v3 0/2] 'part name' subcommand and some robustification Rasmus Villemoes @ 2025-11-10 20:54 ` Rasmus Villemoes 2025-11-11 5:18 ` Anshul Dalal 2025-11-10 20:54 ` [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand Rasmus Villemoes ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2025-11-10 20:54 UTC (permalink / raw) To: u-boot; +Cc: Quentin Schulz, Tom Rini, Rasmus Villemoes Not all ->get_info implementations necessarily populate all the string members of struct disk_partition. Currently, only part_get_info_by_type() (and thereby part_get_info) ensure that the uuid strings are initialized; part_get_info_by_type() and part_get_info_by_uuid() do not. In fact, the latter could lead to a false positive match - if the ->get_info backend does not populate info->uuid, stale contents in info could cause the strncasecmp() to succeed. None of the functions currently ensure that the ->name and ->type strings are initialized. Instead of forcing all callers of any of these functions to pre-initialize info, or all implementations of the ->get_info method to ensure there are valid C strings in all four fields, create a small helper function and factor all invocations of ->get_info through that. This also consolidates the -ENOSYS check and standardizes on using log_debug() for reporting absence, instead of the current mix of PRINTF and log_debug(). It does mean we have to special-case -ENOSYS in the error cases inside the loops in the _by_uuid() and _by_name() functions, but it's still a net win in #LOC. Acked-by: Quentin Schulz <quentin.schulz@cherry.de> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> --- cmd/gpt.c | 4 ++-- disk/part.c | 63 +++++++++++++++++++++++++++++--------------------- include/part.h | 16 +++++++++++++ 3 files changed, 55 insertions(+), 28 deletions(-) diff --git a/cmd/gpt.c b/cmd/gpt.c index e18e5036a06..84221881c39 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -724,7 +724,7 @@ static int gpt_enumerate(struct blk_desc *desc) continue; for (i = 1; i < part_drv->max_entries; i++) { - ret = part_drv->get_info(desc, i, &pinfo); + ret = part_driver_get_info(part_drv, desc, i, &pinfo); if (ret) continue; @@ -820,7 +820,7 @@ static int gpt_setenv(struct blk_desc *desc, const char *name) int i; for (i = 1; i < part_drv->max_entries; i++) { - ret = part_drv->get_info(desc, i, &pinfo); + ret = part_driver_get_info(part_drv, desc, i, &pinfo); if (ret) continue; diff --git a/disk/part.c b/disk/part.c index be2b45d5a29..49a0fca6b89 100644 --- a/disk/part.c +++ b/disk/part.c @@ -72,6 +72,28 @@ struct part_driver *part_driver_lookup_type(struct blk_desc *desc) return NULL; } +static void disk_partition_clr(struct disk_partition *info) +{ + /* The common case is no UUID support */ + disk_partition_clr_uuid(info); + disk_partition_clr_type_guid(info); + info->name[0] = '\0'; + info->type[0] = '\0'; +} + +int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part, + struct disk_partition *info) +{ + if (!drv->get_info) { + log_debug("## Driver %s does not have the get_info() method\n", + drv->name); + return -ENOSYS; + } + + disk_partition_clr(info); + return drv->get_info(desc, part, info); +} + int part_get_type_by_name(const char *name) { struct part_driver *drv = @@ -322,12 +344,9 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type, struct disk_partition *info) { struct part_driver *drv; + int ret = -ENOENT; if (blk_enabled()) { - /* The common case is no UUID support */ - disk_partition_clr_uuid(info); - disk_partition_clr_type_guid(info); - if (part_type == PART_TYPE_UNKNOWN) { drv = part_driver_lookup_type(desc); } else { @@ -339,18 +358,16 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type, desc->part_type); return -EPROTONOSUPPORT; } - if (!drv->get_info) { - PRINTF("## Driver %s does not have the get_info() method\n", - drv->name); - return -ENOSYS; - } - if (drv->get_info(desc, part, info) == 0) { + + ret = part_driver_get_info(drv, desc, part, info); + if (ret && ret != -ENOSYS) { + ret = -ENOENT; + } else { PRINTF("## Valid %s partition found ##\n", drv->name); - return 0; } } - return -ENOENT; + return ret; } int part_get_info(struct blk_desc *desc, int part, @@ -657,15 +674,12 @@ int part_get_info_by_name(struct blk_desc *desc, const char *name, if (!part_drv) return -1; - if (!part_drv->get_info) { - log_debug("## Driver %s does not have the get_info() method\n", - part_drv->name); - return -ENOSYS; - } - for (i = 1; i < part_drv->max_entries; i++) { - ret = part_drv->get_info(desc, i, info); + ret = part_driver_get_info(part_drv, desc, i, info); if (ret != 0) { + /* -ENOSYS means no ->get_info method. */ + if (ret == -ENOSYS) + return ret; /* * Partition with this index can't be obtained, but * further partitions might be, so keep checking. @@ -695,15 +709,12 @@ int part_get_info_by_uuid(struct blk_desc *desc, const char *uuid, if (!part_drv) return -1; - if (!part_drv->get_info) { - log_debug("## Driver %s does not have the get_info() method\n", - part_drv->name); - return -ENOSYS; - } - for (i = 1; i < part_drv->max_entries; i++) { - ret = part_drv->get_info(desc, i, info); + ret = part_driver_get_info(part_drv, desc, i, info); if (ret != 0) { + /* -ENOSYS means no ->get_info method. */ + if (ret == -ENOSYS) + return ret; /* * Partition with this index can't be obtained, but * further partitions might be, so keep checking. diff --git a/include/part.h b/include/part.h index 6caaa6526aa..d940f8b5d0e 100644 --- a/include/part.h +++ b/include/part.h @@ -509,6 +509,22 @@ struct part_driver { int (*test)(struct blk_desc *desc); }; +/** + * part_driver_get_info() - Call the part_driver's get_info method + * + * On success, string members of info are guaranteed to be properly + * initialized, though they may be empty. + * + * @drv: Partition driver + * @desc: Block device descriptor + * @part: Partition number to read + * @info: Returned partition information + * + * Return: 0 on success, negative errno on failure. + */ +int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part, + struct disk_partition *info); + /* Declare a new U-Boot partition 'driver' */ #define U_BOOT_PART_TYPE(__name) \ ll_entry_declare(struct part_driver, __name, part_driver) -- 2.51.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info 2025-11-10 20:54 ` [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info Rasmus Villemoes @ 2025-11-11 5:18 ` Anshul Dalal 2025-11-11 7:58 ` Rasmus Villemoes 0 siblings, 1 reply; 10+ messages in thread From: Anshul Dalal @ 2025-11-11 5:18 UTC (permalink / raw) To: Rasmus Villemoes, u-boot; +Cc: Quentin Schulz, Tom Rini Hi Rasmus, On Tue Nov 11, 2025 at 2:24 AM IST, Rasmus Villemoes wrote: > Not all ->get_info implementations necessarily populate all the string > members of struct disk_partition. > > Currently, only part_get_info_by_type() (and thereby part_get_info) > ensure that the uuid strings are initialized; part_get_info_by_type() > and part_get_info_by_uuid() do not. In fact, the latter could lead to > a false positive match - if the ->get_info backend does not populate > info->uuid, stale contents in info could cause the strncasecmp() to > succeed. > > None of the functions currently ensure that the ->name and ->type > strings are initialized. > > Instead of forcing all callers of any of these functions to > pre-initialize info, or all implementations of the ->get_info method > to ensure there are valid C strings in all four fields, create a small > helper function and factor all invocations of ->get_info through that. > > This also consolidates the -ENOSYS check and standardizes on using > log_debug() for reporting absence, instead of the current mix of > PRINTF and log_debug(). It does mean we have to special-case -ENOSYS > in the error cases inside the loops in the _by_uuid() and _by_name() > functions, but it's still a net win in #LOC. > > Acked-by: Quentin Schulz <quentin.schulz@cherry.de> > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > --- > cmd/gpt.c | 4 ++-- > disk/part.c | 63 +++++++++++++++++++++++++++++--------------------- > include/part.h | 16 +++++++++++++ > 3 files changed, 55 insertions(+), 28 deletions(-) > > diff --git a/cmd/gpt.c b/cmd/gpt.c > index e18e5036a06..84221881c39 100644 > --- a/cmd/gpt.c > +++ b/cmd/gpt.c > @@ -724,7 +724,7 @@ static int gpt_enumerate(struct blk_desc *desc) > continue; > > for (i = 1; i < part_drv->max_entries; i++) { > - ret = part_drv->get_info(desc, i, &pinfo); > + ret = part_driver_get_info(part_drv, desc, i, &pinfo); > if (ret) > continue; > > @@ -820,7 +820,7 @@ static int gpt_setenv(struct blk_desc *desc, const char *name) > int i; > > for (i = 1; i < part_drv->max_entries; i++) { > - ret = part_drv->get_info(desc, i, &pinfo); > + ret = part_driver_get_info(part_drv, desc, i, &pinfo); > if (ret) > continue; > > diff --git a/disk/part.c b/disk/part.c > index be2b45d5a29..49a0fca6b89 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -72,6 +72,28 @@ struct part_driver *part_driver_lookup_type(struct blk_desc *desc) > return NULL; > } > > +static void disk_partition_clr(struct disk_partition *info) > +{ > + /* The common case is no UUID support */ > + disk_partition_clr_uuid(info); > + disk_partition_clr_type_guid(info); > + info->name[0] = '\0'; > + info->type[0] = '\0'; > +} > + > +int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part, > + struct disk_partition *info) > +{ > + if (!drv->get_info) { > + log_debug("## Driver %s does not have the get_info() method\n", > + drv->name); > + return -ENOSYS; > + } > + > + disk_partition_clr(info); > + return drv->get_info(desc, part, info); > +} > + > int part_get_type_by_name(const char *name) > { > struct part_driver *drv = > @@ -322,12 +344,9 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type, > struct disk_partition *info) > { > struct part_driver *drv; > + int ret = -ENOENT; > > if (blk_enabled()) { > - /* The common case is no UUID support */ > - disk_partition_clr_uuid(info); > - disk_partition_clr_type_guid(info); > - > if (part_type == PART_TYPE_UNKNOWN) { > drv = part_driver_lookup_type(desc); > } else { > @@ -339,18 +358,16 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type, > desc->part_type); > return -EPROTONOSUPPORT; > } > - if (!drv->get_info) { > - PRINTF("## Driver %s does not have the get_info() method\n", > - drv->name); > - return -ENOSYS; > - } > - if (drv->get_info(desc, part, info) == 0) { > + > + ret = part_driver_get_info(drv, desc, part, info); > + if (ret && ret != -ENOSYS) { > + ret = -ENOENT; Why are we overwriting the err code from part_driver_get_info here? Regards, Anshul > + } else { > PRINTF("## Valid %s partition found ##\n", drv->name); > - return 0; > } > } > > - return -ENOENT; > + return ret; > } > > int part_get_info(struct blk_desc *desc, int part, > @@ -657,15 +674,12 @@ int part_get_info_by_name(struct blk_desc *desc, const char *name, > if (!part_drv) > return -1; > > - if (!part_drv->get_info) { > - log_debug("## Driver %s does not have the get_info() method\n", > - part_drv->name); > - return -ENOSYS; > - } > - > for (i = 1; i < part_drv->max_entries; i++) { > - ret = part_drv->get_info(desc, i, info); > + ret = part_driver_get_info(part_drv, desc, i, info); > if (ret != 0) { > + /* -ENOSYS means no ->get_info method. */ > + if (ret == -ENOSYS) > + return ret; > /* > * Partition with this index can't be obtained, but > * further partitions might be, so keep checking. > @@ -695,15 +709,12 @@ int part_get_info_by_uuid(struct blk_desc *desc, const char *uuid, > if (!part_drv) > return -1; > > - if (!part_drv->get_info) { > - log_debug("## Driver %s does not have the get_info() method\n", > - part_drv->name); > - return -ENOSYS; > - } > - > for (i = 1; i < part_drv->max_entries; i++) { > - ret = part_drv->get_info(desc, i, info); > + ret = part_driver_get_info(part_drv, desc, i, info); > if (ret != 0) { > + /* -ENOSYS means no ->get_info method. */ > + if (ret == -ENOSYS) > + return ret; > /* > * Partition with this index can't be obtained, but > * further partitions might be, so keep checking. > diff --git a/include/part.h b/include/part.h > index 6caaa6526aa..d940f8b5d0e 100644 > --- a/include/part.h > +++ b/include/part.h > @@ -509,6 +509,22 @@ struct part_driver { > int (*test)(struct blk_desc *desc); > }; > > +/** > + * part_driver_get_info() - Call the part_driver's get_info method > + * > + * On success, string members of info are guaranteed to be properly > + * initialized, though they may be empty. > + * > + * @drv: Partition driver > + * @desc: Block device descriptor > + * @part: Partition number to read > + * @info: Returned partition information > + * > + * Return: 0 on success, negative errno on failure. > + */ > +int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part, > + struct disk_partition *info); > + > /* Declare a new U-Boot partition 'driver' */ > #define U_BOOT_PART_TYPE(__name) \ > ll_entry_declare(struct part_driver, __name, part_driver) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info 2025-11-11 5:18 ` Anshul Dalal @ 2025-11-11 7:58 ` Rasmus Villemoes 0 siblings, 0 replies; 10+ messages in thread From: Rasmus Villemoes @ 2025-11-11 7:58 UTC (permalink / raw) To: Anshul Dalal; +Cc: u-boot, Quentin Schulz, Tom Rini On Tue, Nov 11 2025, Anshul Dalal <anshuld@ti.com> wrote: >> int part_get_type_by_name(const char *name) >> { >> struct part_driver *drv = >> @@ -322,12 +344,9 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type, >> struct disk_partition *info) >> { >> struct part_driver *drv; >> + int ret = -ENOENT; >> >> if (blk_enabled()) { >> - /* The common case is no UUID support */ >> - disk_partition_clr_uuid(info); >> - disk_partition_clr_type_guid(info); >> - >> if (part_type == PART_TYPE_UNKNOWN) { >> drv = part_driver_lookup_type(desc); >> } else { >> @@ -339,18 +358,16 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type, >> desc->part_type); >> return -EPROTONOSUPPORT; >> } >> - if (!drv->get_info) { >> - PRINTF("## Driver %s does not have the get_info() method\n", >> - drv->name); >> - return -ENOSYS; >> - } >> - if (drv->get_info(desc, part, info) == 0) { >> + >> + ret = part_driver_get_info(drv, desc, part, info); >> + if (ret && ret != -ENOSYS) { >> + ret = -ENOENT; > > Why are we overwriting the err code from part_driver_get_info here? That's essentially what the code did originally (it only returned 0 in case ->get_info returned 0, otherwise it fell through to the "return -ENOENT" at the bottom). And it turns out that that behaviour is expected by lots of tests; when I inadvertently changed that logic in v2 to actually propagate whatever error ->get_info returned, lots of tests broke: https://lore.kernel.org/u-boot/20251107201927.GA2243313@bill-the-cat/ And this is the only difference between v2 and v3, I tweaked this so it would preserve that "either success or -ENOENT [or -ENOSYS]" behaviour. Rasmus ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand 2025-11-10 20:54 [PATCH v3 0/2] 'part name' subcommand and some robustification Rasmus Villemoes 2025-11-10 20:54 ` [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info Rasmus Villemoes @ 2025-11-10 20:54 ` Rasmus Villemoes 2025-11-11 5:25 ` Anshul Dalal 2025-11-11 5:07 ` [PATCH v3 0/2] 'part name' subcommand and some robustification Anshul Dalal 2025-11-18 20:32 ` Tom Rini 3 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2025-11-10 20:54 UTC (permalink / raw) To: u-boot; +Cc: Quentin Schulz, Tom Rini, Rasmus Villemoes This is a natural buddy to the existing "part number", allowing one to get the partition name for a given partition number. Acked-by: Quentin Schulz <quentin.schulz@cherry.de> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> --- cmd/part.c | 16 +++++++++++++++- doc/usage/cmd/part.rst | 13 +++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/cmd/part.c b/cmd/part.c index db7bc5819c0..975a0a08a99 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -25,7 +25,8 @@ enum cmd_part_info { CMD_PART_INFO_START = 0, CMD_PART_INFO_SIZE, - CMD_PART_INFO_NUMBER + CMD_PART_INFO_NUMBER, + CMD_PART_INFO_NAME, }; static int do_part_uuid(int argc, char *const argv[]) @@ -154,6 +155,9 @@ static int do_part_info(int argc, char *const argv[], enum cmd_part_info param) case CMD_PART_INFO_NUMBER: snprintf(buf, sizeof(buf), "0x%x", part); break; + case CMD_PART_INFO_NAME: + snprintf(buf, sizeof(buf), "%s", info.name); + break; default: printf("** Unknown cmd_part_info value: %d\n", param); return 1; @@ -182,6 +186,11 @@ static int do_part_number(int argc, char *const argv[]) return do_part_info(argc, argv, CMD_PART_INFO_NUMBER); } +static int do_part_name(int argc, char *const argv[]) +{ + return do_part_info(argc, argv, CMD_PART_INFO_NAME); +} + static int do_part_set(int argc, char *const argv[]) { const char *devname, *partstr, *typestr; @@ -273,6 +282,8 @@ static int do_part(struct cmd_tbl *cmdtp, int flag, int argc, return do_part_size(argc - 2, argv + 2); else if (!strcmp(argv[1], "number")) return do_part_number(argc - 2, argv + 2); + else if (!strcmp(argv[1], "name")) + return do_part_name(argc - 2, argv + 2); else if (!strcmp(argv[1], "types")) return do_part_types(argc - 2, argv + 2); else if (!strcmp(argv[1], "set")) @@ -305,6 +316,9 @@ U_BOOT_CMD( "part number <interface> <dev> <part> <varname>\n" " - set environment variable to the partition number using the partition name\n" " part must be specified as partition name\n" + "part name <interface> <dev> <part> <varname>\n" + " - set environment variable to the partition name using the partition number\n" + " part must be specified as partition number\n" #ifdef CONFIG_PARTITION_TYPE_GUID "part type <interface> <dev>:<part>\n" " - print partition type\n" diff --git a/doc/usage/cmd/part.rst b/doc/usage/cmd/part.rst index 72f5d8b8de7..299f2ac15c5 100644 --- a/doc/usage/cmd/part.rst +++ b/doc/usage/cmd/part.rst @@ -16,6 +16,7 @@ Synopsis part start <interface> <dev> <part> <varname> part size <interface> <dev> <part> <varname> part number <interface> <dev> <part> <varname> + part name <interface> <dev> <part> <varname> part set <interface> <dev> <part> <type> part type <interface> <dev>:<part> [varname] part types @@ -86,6 +87,18 @@ part must be specified as partition name. varname a variable to store the current partition number value into +The 'part name' command sets an environment variable to the partition name using the partition number, +part must be specified as partition number. + + interface + interface for accessing the block device (mmc, sata, scsi, usb, ....) + dev + device number + part + partition number + varname + a variable to store the current partition name into + The 'part set' command sets the type of a partition. This is useful when autodetection fails or does not do the correct thing: -- 2.51.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand 2025-11-10 20:54 ` [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand Rasmus Villemoes @ 2025-11-11 5:25 ` Anshul Dalal 2025-11-11 8:00 ` Rasmus Villemoes 0 siblings, 1 reply; 10+ messages in thread From: Anshul Dalal @ 2025-11-11 5:25 UTC (permalink / raw) To: Rasmus Villemoes, u-boot; +Cc: Quentin Schulz, Tom Rini On Tue Nov 11, 2025 at 2:24 AM IST, Rasmus Villemoes wrote: > This is a natural buddy to the existing "part number", allowing one to > get the partition name for a given partition number. > > Acked-by: Quentin Schulz <quentin.schulz@cherry.de> > Signed-off-by: Rasmus Villemoes <ravi@prevas.dk> > --- > cmd/part.c | 16 +++++++++++++++- > doc/usage/cmd/part.rst | 13 +++++++++++++ > 2 files changed, 28 insertions(+), 1 deletion(-) > > diff --git a/cmd/part.c b/cmd/part.c > index db7bc5819c0..975a0a08a99 100644 > --- a/cmd/part.c > +++ b/cmd/part.c > @@ -25,7 +25,8 @@ > enum cmd_part_info { > CMD_PART_INFO_START = 0, > CMD_PART_INFO_SIZE, > - CMD_PART_INFO_NUMBER > + CMD_PART_INFO_NUMBER, > + CMD_PART_INFO_NAME, > }; > > static int do_part_uuid(int argc, char *const argv[]) > @@ -154,6 +155,9 @@ static int do_part_info(int argc, char *const argv[], enum cmd_part_info param) > case CMD_PART_INFO_NUMBER: > snprintf(buf, sizeof(buf), "0x%x", part); > break; > + case CMD_PART_INFO_NAME: > + snprintf(buf, sizeof(buf), "%s", info.name); > + break; > default: > printf("** Unknown cmd_part_info value: %d\n", param); > return 1; > @@ -182,6 +186,11 @@ static int do_part_number(int argc, char *const argv[]) > return do_part_info(argc, argv, CMD_PART_INFO_NUMBER); > } > > +static int do_part_name(int argc, char *const argv[]) > +{ > + return do_part_info(argc, argv, CMD_PART_INFO_NAME); > +} > + > static int do_part_set(int argc, char *const argv[]) > { > const char *devname, *partstr, *typestr; > @@ -273,6 +282,8 @@ static int do_part(struct cmd_tbl *cmdtp, int flag, int argc, > return do_part_size(argc - 2, argv + 2); > else if (!strcmp(argv[1], "number")) > return do_part_number(argc - 2, argv + 2); > + else if (!strcmp(argv[1], "name")) > + return do_part_name(argc - 2, argv + 2); > else if (!strcmp(argv[1], "types")) > return do_part_types(argc - 2, argv + 2); > else if (!strcmp(argv[1], "set")) > @@ -305,6 +316,9 @@ U_BOOT_CMD( > "part number <interface> <dev> <part> <varname>\n" > " - set environment variable to the partition number using the partition name\n" > " part must be specified as partition name\n" > + "part name <interface> <dev> <part> <varname>\n" Nit: Shouldn't this be part name <interface> <dev> <part> [varname] instead? > + " - set environment variable to the partition name using the partition number\n" > + " part must be specified as partition number\n" > #ifdef CONFIG_PARTITION_TYPE_GUID > "part type <interface> <dev>:<part>\n" > " - print partition type\n" > diff --git a/doc/usage/cmd/part.rst b/doc/usage/cmd/part.rst > index 72f5d8b8de7..299f2ac15c5 100644 > --- a/doc/usage/cmd/part.rst > +++ b/doc/usage/cmd/part.rst > @@ -16,6 +16,7 @@ Synopsis > part start <interface> <dev> <part> <varname> > part size <interface> <dev> <part> <varname> > part number <interface> <dev> <part> <varname> > + part name <interface> <dev> <part> <varname> Same here. Regards, Anshul > part set <interface> <dev> <part> <type> > part type <interface> <dev>:<part> [varname] > part types > @@ -86,6 +87,18 @@ part must be specified as partition name. > varname > a variable to store the current partition number value into > > +The 'part name' command sets an environment variable to the partition name using the partition number, > +part must be specified as partition number. > + > + interface > + interface for accessing the block device (mmc, sata, scsi, usb, ....) > + dev > + device number > + part > + partition number > + varname > + a variable to store the current partition name into > + > The 'part set' command sets the type of a partition. This is useful when > autodetection fails or does not do the correct thing: > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand 2025-11-11 5:25 ` Anshul Dalal @ 2025-11-11 8:00 ` Rasmus Villemoes 2025-11-11 17:27 ` Quentin Schulz 0 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2025-11-11 8:00 UTC (permalink / raw) To: Anshul Dalal; +Cc: u-boot, Quentin Schulz, Tom Rini On Tue, Nov 11 2025, Anshul Dalal <anshuld@ti.com> wrote: >> static int do_part_set(int argc, char *const argv[]) >> { >> const char *devname, *partstr, *typestr; >> @@ -273,6 +282,8 @@ static int do_part(struct cmd_tbl *cmdtp, int flag, int argc, >> return do_part_size(argc - 2, argv + 2); >> else if (!strcmp(argv[1], "number")) >> return do_part_number(argc - 2, argv + 2); >> + else if (!strcmp(argv[1], "name")) >> + return do_part_name(argc - 2, argv + 2); >> else if (!strcmp(argv[1], "types")) >> return do_part_types(argc - 2, argv + 2); >> else if (!strcmp(argv[1], "set")) >> @@ -305,6 +316,9 @@ U_BOOT_CMD( >> "part number <interface> <dev> <part> <varname>\n" >> " - set environment variable to the partition number using the partition name\n" >> " part must be specified as partition name\n" >> + "part name <interface> <dev> <part> <varname>\n" > > Nit: Shouldn't this be part name <interface> <dev> <part> [varname] > instead? > Yes, probably, I did notice that, but I preferred being consistent with the existing cases which also do not indicate varname as optional. Rasmus ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand 2025-11-11 8:00 ` Rasmus Villemoes @ 2025-11-11 17:27 ` Quentin Schulz 0 siblings, 0 replies; 10+ messages in thread From: Quentin Schulz @ 2025-11-11 17:27 UTC (permalink / raw) To: Rasmus Villemoes, Anshul Dalal; +Cc: u-boot, Tom Rini Hi Rasmus, On 11/11/25 9:00 AM, Rasmus Villemoes wrote: > On Tue, Nov 11 2025, Anshul Dalal <anshuld@ti.com> wrote: > >>> static int do_part_set(int argc, char *const argv[]) >>> { >>> const char *devname, *partstr, *typestr; >>> @@ -273,6 +282,8 @@ static int do_part(struct cmd_tbl *cmdtp, int flag, int argc, >>> return do_part_size(argc - 2, argv + 2); >>> else if (!strcmp(argv[1], "number")) >>> return do_part_number(argc - 2, argv + 2); >>> + else if (!strcmp(argv[1], "name")) >>> + return do_part_name(argc - 2, argv + 2); >>> else if (!strcmp(argv[1], "types")) >>> return do_part_types(argc - 2, argv + 2); >>> else if (!strcmp(argv[1], "set")) >>> @@ -305,6 +316,9 @@ U_BOOT_CMD( >>> "part number <interface> <dev> <part> <varname>\n" >>> " - set environment variable to the partition number using the partition name\n" >>> " part must be specified as partition name\n" >>> + "part name <interface> <dev> <part> <varname>\n" >> >> Nit: Shouldn't this be part name <interface> <dev> <part> [varname] >> instead? >> > > Yes, probably, I did notice that, but I preferred being consistent with > the existing cases which also do not indicate varname as optional. > Let's not be consistent with a mistake if it can be fixed without breaking compatibility. From what I gathered, it's a simple help text (and maybe doc) update, so i think it's better to fix it. Cheers, Quentin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] 'part name' subcommand and some robustification 2025-11-10 20:54 [PATCH v3 0/2] 'part name' subcommand and some robustification Rasmus Villemoes 2025-11-10 20:54 ` [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info Rasmus Villemoes 2025-11-10 20:54 ` [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand Rasmus Villemoes @ 2025-11-11 5:07 ` Anshul Dalal 2025-11-18 20:32 ` Tom Rini 3 siblings, 0 replies; 10+ messages in thread From: Anshul Dalal @ 2025-11-11 5:07 UTC (permalink / raw) To: Rasmus Villemoes, u-boot; +Cc: Quentin Schulz, Tom Rini On Tue Nov 11, 2025 at 2:24 AM IST, Rasmus Villemoes wrote: > Implement a "part name" subcommand, mirroring the existing "part > number" subcommand. > > In the discussion for v1 of that, it came up that there's a bit of > inconsistency in how much and what one can assume to be initialized in > 'struct disk_partition' after a successful call of one of the > get_info* family of functions. Patch 1/2 tries to consolidate > that by making sure all ->get_info invocations go through a common > helper that at least always initializes the string members. > > Quentin, I've taken the liberty of including your Acks, as the > incremental diff in patch 1 is quite minor, but do speak up if I > should not have done that. > > v3: Make sure part_get_type_by_name() can only ever return 0, -ENOSYS > (in case ->get_info is not implemented) and -ENOENT, rather than > anything that ->get_info might return. > > v2: https://lore.kernel.org/u-boot/20251020121100.1742812-1-ravi@prevas.dk/ > For the entire series: Tested-by: Anshul Dalal <anshuld@ti.com> Logs (on TI AM62x): => part part - disk partition related commands Usage: part uuid <interface> <dev>:<part> - print partition UUID part uuid <interface> <dev>:<part> <varname> - set environment variable to partition UUID part list <interface> <dev> - print a device's partition table part list <interface> <dev> [flags] <varname> - set environment variable to the list of partitions flags can be -bootable (list only bootable partitions) part start <interface> <dev> <part> <varname> - set environment variable to the start of the partition (in blocks) part can be either partition number or partition name part size <interface> <dev> <part> <varname> - set environment variable to the size of the partition (in blocks) part can be either partition number or partition name part number <interface> <dev> <part> <varname> - set environment variable to the partition number using the partition name part must be specified as partition name part name <interface> <dev> <part> <varname> - set environment variable to the partition name using the partition number part must be specified as partition number part set <interface> <dev> type - set partition type for a device part types - list supported partition table types => part list mmc 1 Partition Map for mmc device 1 -- Partition Type: DOS Part Start Sector Num Sectors UUID Type 1 2048 262144 076c4a2a-01 0c Boot 2 264192 417336 076c4a2a-02 83 => part name mmc 1 1 mmcsdb1 => part name mmc 1 2 mmcsdb2 Regards, Anshul > Rasmus Villemoes (2): > disk/part.c: ensure strings in struct disk_partition are valid after > successful get_info > cmd/part.c: implement "part name" subcommand > > cmd/gpt.c | 4 +-- > cmd/part.c | 16 ++++++++++- > disk/part.c | 63 +++++++++++++++++++++++++----------------- > doc/usage/cmd/part.rst | 13 +++++++++ > include/part.h | 16 +++++++++++ > 5 files changed, 83 insertions(+), 29 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] 'part name' subcommand and some robustification 2025-11-10 20:54 [PATCH v3 0/2] 'part name' subcommand and some robustification Rasmus Villemoes ` (2 preceding siblings ...) 2025-11-11 5:07 ` [PATCH v3 0/2] 'part name' subcommand and some robustification Anshul Dalal @ 2025-11-18 20:32 ` Tom Rini 3 siblings, 0 replies; 10+ messages in thread From: Tom Rini @ 2025-11-18 20:32 UTC (permalink / raw) To: u-boot, Rasmus Villemoes; +Cc: Quentin Schulz On Mon, 10 Nov 2025 21:54:09 +0100, Rasmus Villemoes wrote: > Implement a "part name" subcommand, mirroring the existing "part > number" subcommand. > > In the discussion for v1 of that, it came up that there's a bit of > inconsistency in how much and what one can assume to be initialized in > 'struct disk_partition' after a successful call of one of the > get_info* family of functions. Patch 1/2 tries to consolidate > that by making sure all ->get_info invocations go through a common > helper that at least always initializes the string members. > > [...] Applied to u-boot/next, thanks! [1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info commit: 3c2a9475334ae32997b3bce8d24b39c8ffa9fc67 [2/2] cmd/part.c: implement "part name" subcommand commit: 30890051ab23a0293f6404c9a49e86f33e45df66 -- Tom ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-11-18 20:32 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-10 20:54 [PATCH v3 0/2] 'part name' subcommand and some robustification Rasmus Villemoes 2025-11-10 20:54 ` [PATCH v3 1/2] disk/part.c: ensure strings in struct disk_partition are valid after successful get_info Rasmus Villemoes 2025-11-11 5:18 ` Anshul Dalal 2025-11-11 7:58 ` Rasmus Villemoes 2025-11-10 20:54 ` [PATCH v3 2/2] cmd/part.c: implement "part name" subcommand Rasmus Villemoes 2025-11-11 5:25 ` Anshul Dalal 2025-11-11 8:00 ` Rasmus Villemoes 2025-11-11 17:27 ` Quentin Schulz 2025-11-11 5:07 ` [PATCH v3 0/2] 'part name' subcommand and some robustification Anshul Dalal 2025-11-18 20:32 ` Tom Rini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.