Hi Sumit,
On 2026-09-28T06:40:33, Sumit Singh Dhami <[email protected]> wrote:
> cmd: gpt: support setting full 64-bit GPT partition attributes
>
> The 'gpt write' partition list syntax only allows setting the legacy
> BIOS-bootable bit via the 'bootable' key.
>
> Parse an optional 'attrs=<value>' key when CONFIG_PARTITION_ATTR is
> enabled and store it via disk_partition_set_gpt_attr(),
> so that gpt_fill_pte() writes the requested 64-bit attributes value
> into the partition's GPT entry.
>
> Signed-off-by: Sumit Singh Dhami <[email protected]>
>
> cmd/gpt.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> @@ -592,6 +592,17 @@ static int set_gpt_info(struct blk_desc *dev_desc,
> + if (CONFIG_IS_ENABLED(PARTITION_ATTR)) {
> + /* attrs */
> + val = extract_val(tok, "attrs");
> + if (val) { /* 'attrs' is optional */
> + if (extract_env(val, &p))
> + p = val;
> + disk_partition_set_gpt_attr(&parts[i],
> simple_strtoull(p, NULL, 0));
A typo such as attrs=0x800000000000000g or attrs=abc is silently
accepted here and a partially-parsed or zero value is written to the
disk. Since this is a raw bit-field the user cannot easily see
afterwards, please can you pass an end pointer and reject the entry
when it does not point at the terminating NUL, as the uuid/type checks
above do (print an error and goto err with -4)? Parsing into a local
first would also bring this line back under 80 columns.
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> @@ -592,6 +592,17 @@ static int set_gpt_info(struct blk_desc *dev_desc,
> + if (CONFIG_IS_ENABLED(PARTITION_ATTR)) {
> + /* attrs */
> + val = extract_val(tok, "attrs");
Just to check the round trip: 'gpt rename', 'gpt swap' and 'gpt
transpose' all go through create_gpt_partitions_list() to rebuild the
partition string and back through set_gpt_info(), and that function
only emits uuid, name, start, size and bootable. So a table written
with attrs= loses those bits as soon as it is renamed. Now that patch
3 populates gpt_attr on read, I think create_gpt_partitions_list()
should append ',attrs=0x%016llx' when the attribute is set (and
calc_parts_list_len() needs the extra room). Otherwise please at least
note the limitation in the docs. What do you think?
Regards,
Simon
