All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.