[PATCH v2 3/4] cmd: gpt: fix calc_parts_list_len()

[Heinrich Schuchardt]

On 9/4/23 18:48, Simon Glass wrote:
> Hi Heinrich,
> On Sat, 2 Sept 2023 at 01:34, Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>> * Avoid incrementing by moving comma into strlen("uuid_disk=,") and
>>    considering NUL byte.
>> * Appending a UUID only adds UUID_STR_LEN bytes.
>>    Don't count the terminating NUL.
>> * The length of the hexadecimal representation of lba_int is
>>    2 * sizeof(lba_int).
>> * We don't use a 'MiB' postfix but a '0x' prefix.
> 
> Why do we want the 0x prefix?

This patch is about correcting the number of bytes allocated for the 
output buffer. Is does not change the output format.

We want 'gpt write' to understand the output of 'gpt read'.

The ustrtoull(p, &p, 0) function call in set_gpt_info() requires the 
"0x" prefix to understand that size and start are hexadecimal.

See function _parse_integer_fixup_radix().

Best regards

Heinrich

> 
>> * The uuid field is only needed if configured.
>>
>> Fixes: 2fcaa413b3f6 ("gpt: harden set_gpt_info() against non NULL-terminated strings")
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>> v2:
>>          new patch
>> ---
>>   cmd/gpt.c | 33 ++++++++++++++++++++-------------
>>   1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/cmd/gpt.c b/cmd/gpt.c
>> index 092f7defff..e7a53747fc 100644
>> --- a/cmd/gpt.c
>> +++ b/cmd/gpt.c
>> @@ -162,22 +162,29 @@ static bool found_key(const char *str, const char *key)
>>          return result;
>>   }
>>
>> +/**
>> + * calc_parts_list_len() - get size of partition table description
>> + *
>> + * @numparts:  number of partitions
>> + * Return:     string size including terminating NUL
>> + */
>>   static int calc_parts_list_len(int numparts)
>>   {
>> -       int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk=");
>> -       /* for the comma */
>> -       partlistlen++;
>> -
>> -       /* per-partition additions; numparts starts at 1, so this should be correct */
>> -       partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN + 1);
>> +       /* number of hexadecimal digits of the lbaint_t representation */
>> +       const int lbaint_size = 2 * sizeof(lbaint_t);
>> +       int partlistlen;
>> +
>> +       /* media description including terminating NUL */
>> +       partlistlen = strlen("uuid_disk=;") + UUID_STR_LEN + 1;
>> +       /* per-partition descriptions; numparts */
>> +       partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN);
>>          /* see part.h for definition of struct disk_partition */
>> -       partlistlen += numparts * (strlen("start=MiB,") + sizeof(lbaint_t) + 1);
>> -       partlistlen += numparts * (strlen("size=MiB,") + sizeof(lbaint_t) + 1);
>> -       partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1);
>> -       /* for the terminating null */
>> -       partlistlen++;
>> -       debug("Length of partitions_list is %d for %d partitions\n", partlistlen,
>> -             numparts);
>> +       partlistlen += numparts * (strlen("start=0x,") + lbaint_size);
>> +       partlistlen += numparts * (strlen("size=0x,") + lbaint_size);
>> +       if (IS_ENABLED(CONFIG_PARTITION_UUIDS))
>> +               partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN);
>> +       debug("Length of partitions_list is %d for %d partitions\n",
>> +             partlistlen, numparts);
>>          return partlistlen;
>>   }
>>
>> --
>> 2.40.1
>>
> 
> Regards,
> Simon