* [PATCH] cmd/part.c: implement "part name" subcommand
@ 2025-10-16 12:52 Rasmus Villemoes
2025-10-16 13:28 ` Quentin Schulz
0 siblings, 1 reply; 4+ messages in thread
From: Rasmus Villemoes @ 2025-10-16 12:52 UTC (permalink / raw)
To: u-boot; +Cc: 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.
Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
cmd/part.c | 16 +++++++++++++++-
1 file changed, 15 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"
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cmd/part.c: implement "part name" subcommand
2025-10-16 12:52 [PATCH] cmd/part.c: implement "part name" subcommand Rasmus Villemoes
@ 2025-10-16 13:28 ` Quentin Schulz
2025-10-17 8:22 ` Rasmus Villemoes
0 siblings, 1 reply; 4+ messages in thread
From: Quentin Schulz @ 2025-10-16 13:28 UTC (permalink / raw)
To: Rasmus Villemoes, u-boot; +Cc: Tom Rini
Hi Rasmus,
On 10/16/25 2:52 PM, 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.
>
Can you update the documentation in doc/usage/cmd/part.rst as well?
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> ---
> cmd/part.c | 16 +++++++++++++++-
> 1 file changed, 15 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);
Is it possible info.name is never set by the fs/blk driver? Should we
initialize this field?
Cheers,
Quentin
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cmd/part.c: implement "part name" subcommand
2025-10-16 13:28 ` Quentin Schulz
@ 2025-10-17 8:22 ` Rasmus Villemoes
2025-10-17 9:09 ` Quentin Schulz
0 siblings, 1 reply; 4+ messages in thread
From: Rasmus Villemoes @ 2025-10-17 8:22 UTC (permalink / raw)
To: Quentin Schulz; +Cc: u-boot, Tom Rini
On Thu, Oct 16 2025, Quentin Schulz <quentin.schulz@cherry.de> wrote:
> Hi Rasmus,
>
> On 10/16/25 2:52 PM, 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.
>>
>
> Can you update the documentation in doc/usage/cmd/part.rst as well?
Will do, thanks.
>> cmd/part.c | 16 +++++++++++++++-
>> 1 file changed, 15 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);
>
> Is it possible info.name is never set by the fs/blk driver? Should we
> initialize this field?
>
So I took a look, and it seems that all but part_amiga.c do initialize
it.
- iso and dos uses part_set_generic_name()
- mac and efi have real data from the partition table to put in
- mtd also initializes name, either from a name defined in mtdparts or a
size@offset generated one
- ubi uses the volume name
I suppose we should make sure info.name is zero-initialized (or, at
least, that info.name[0] is 0 so there's a valid C string). But I think
that shouldn't have to be the responsibility of every caller of
part_get_info(). IOW, I think we should just add
info->name[0] = '\0';
info->type[0] = '\0';
into part_get_info_by_type(), and then that's a separate patch. WDYT?
Rasmus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cmd/part.c: implement "part name" subcommand
2025-10-17 8:22 ` Rasmus Villemoes
@ 2025-10-17 9:09 ` Quentin Schulz
0 siblings, 0 replies; 4+ messages in thread
From: Quentin Schulz @ 2025-10-17 9:09 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: u-boot, Tom Rini
Hi Rasmus,
On 10/17/25 10:22 AM, Rasmus Villemoes wrote:
> On Thu, Oct 16 2025, Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
>> Hi Rasmus,
>>
>> On 10/16/25 2:52 PM, Rasmus Villemoes wrote:
[...]
>>> cmd/part.c | 16 +++++++++++++++-
>>> 1 file changed, 15 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);
>>
>> Is it possible info.name is never set by the fs/blk driver? Should we
>> initialize this field?
>>
>
> So I took a look, and it seems that all but part_amiga.c do initialize
> it.
>
> - iso and dos uses part_set_generic_name()
>
> - mac and efi have real data from the partition table to put in
>
> - mtd also initializes name, either from a name defined in mtdparts or a
> size@offset generated one
>
> - ubi uses the volume name
>
> I suppose we should make sure info.name is zero-initialized (or, at
> least, that info.name[0] is 0 so there's a valid C string). But I think
> that shouldn't have to be the responsibility of every caller of
> part_get_info(). IOW, I think we should just add
>
> info->name[0] = '\0';
> info->type[0] = '\0';
>
> into part_get_info_by_type(), and then that's a separate patch. WDYT?
>
Yeah, something like disk_partition_clr_name and disk_partition_clr_type
for example and call it around the other disk_partition_clr_uuid() calls
in part_get_info_by_type.
I think this should also be done by *any* caller of drv->get_info? So
likely in part_get_info_by_uuid as well as part_get_info_by_name. I'm
also wondering if we shouldn't clear every member when iterating with
the same disk_partition, e.g. in part_get_info_by_name,
part_get_info_by_uuid, part_get_bootable so that we don't have leftovers
from previous failed loops? Essentially what part_get_info_whole_disk()
does (though it's missing an init for sys_ind and type_flags). Mmmm but
that would mean also patching the loop in cmd/part.c:do_part_list() for
example. Or... maybe the whole thing should be a responsibility of the
get_info implementer (the part_driver) so that calling get_info is
always "safe"?
That would be a separate patch yes, but likely before this one? Or have
at least info.name[0] = '\0'; before calling part_get_info*() in
do_part_info()?
Pfew.. seems like a lot to do here :/
Quentin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-17 9:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-16 12:52 [PATCH] cmd/part.c: implement "part name" subcommand Rasmus Villemoes
2025-10-16 13:28 ` Quentin Schulz
2025-10-17 8:22 ` Rasmus Villemoes
2025-10-17 9:09 ` Quentin Schulz
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.