Ilias Apalodimas <ilias.apalodi...@linaro.org> writes:
Hello Ilias,
> On Mon, 16 Jun 2025 at 17:08, Javier Martinez Canillas
> <javi...@redhat.com> wrote:
>>
>> Heinrich Schuchardt <xypron.g...@gmx.de> writes:
>>
>> Thanks Ilias and Heinrich for your review / feedback.
>>
>> > On 16.06.25 15:32, Ilias Apalodimas wrote:
>> >> On Fri, 13 Jun 2025 at 11:58, Javier Martinez Canillas
>> >> <javi...@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 <javi...@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().
>> >
>>
>> I agree. This allows to have for example flicker-free boot output. But
>> also, I tried to follow the convention I noticed in this file and AFAICT
>> most messages printouts were using just log_debug().
>
> Ah yes, Heinrich is right, I forgot that this is going to be plugged
> in an EFI protocol
>
Cool, does it mean that I've your r-b tag for this patch too ?
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
