[PATCH 1/3] disk: efi: Move logic to get a GPT entry into a helper function

[Heinrich Schuchardt]

On 16.06.25 15:32, Ilias Apalodimas wrote:
> On Fri, 13 Jun 2025 at 11:58, Javier Martinez Canillas
> <javierm at redhat.com> wrote:
>>
>> Factor out the logic to get the Partition Table Entry (PTE) of a given
>> partition into a helper function, since it could be used by other code.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm at redhat.com>
>> ---
>>
>>   disk/part_efi.c | 67 +++++++++++++++++++++++++++++--------------------
>>   1 file changed, 40 insertions(+), 27 deletions(-)
>>
>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>> index 932d058c184c..c7d79e253958 100644
>> --- a/disk/part_efi.c
>> +++ b/disk/part_efi.c
>> @@ -216,6 +216,34 @@ int get_disk_guid(struct blk_desc *desc, char *guid)
>>          return 0;
>>   }
>>
>> +static int part_get_gpt_pte(struct blk_desc *desc, int part, gpt_entry *gpt_e)
>> +{
>> +       ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1, desc->blksz);
>> +       gpt_entry *gpt_pte = NULL;
>> +
>> +       /* "part" argument must be at least 1 */
>> +       if (part < 1) {
>> +               log_debug("Invalid Argument(s)\n");
> 
> I think this should be a log_err an match the comment above

Screen output during a running EFI application like GRUB should be 
avoided as output will no longer match the expected cursor positioning 
for the EFI console.

Please, stick to log_debug().

Shell commands like gpt should write a message based on error codes.

Best regards

Heinrich

> 
>> +               return -EINVAL;
>> +       }
>> +
>> +       /* This function validates AND fills in the GPT header and PTE */
>> +       if (find_valid_gpt(desc, gpt_head, &gpt_pte) != 1)
>> +               return -EINVAL;
>> +
>> +       if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
>> +           !is_pte_valid(&gpt_pte[part - 1])) {
>> +               log_debug("Invalid partition number %d\n", part);
> 
> I haven't read the reat yet, but isn't this something we want visible
> as a normal error?
> 
>> +               free(gpt_pte);
>> +               return -EPERM;
>> +       }
>> +
>> +       memcpy(gpt_e, &gpt_pte[part - 1], sizeof(*gpt_e));
>> +
>> +       free(gpt_pte);
>> +       return 0;
>> +}
>> +
>>   static void __maybe_unused part_print_efi(struct blk_desc *desc)
>>   {
>>          ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1, desc->blksz);
>> @@ -261,45 +289,32 @@ static void __maybe_unused part_print_efi(struct blk_desc *desc)
>>   static int __maybe_unused part_get_info_efi(struct blk_desc *desc, int part,
>>                                              struct disk_partition *info)
> [..]
> 
> Rest looks good
> 
> Thanks
> /Ilias