Re: [PATCH] Add %X option to printf functions.

[Javier Martinez Canillas]

On 9/20/19 2:57 PM, Daniel Kiper wrote:
> On Thu, Sep 19, 2019 at 02:00:31PM +0200, Javier Martinez Canillas wrote:
>> Hello Daniel,
>>
>> Thanks for the quick feedback.
>>
>> On 9/18/19 3:00 PM, Daniel Kiper wrote:
>>> On Tue, Sep 17, 2019 at 05:54:44PM +0200, Javier Martinez Canillas wrote:
>>>> From: Paulo Flabiano Smorigo <address@hidden>
>>>>
>>>> Signed-off-by: Paulo Flabiano Smorigo <address@hidden>
>>>> Signed-off-by: Javier Martinez Canillas <address@hidden>
>>>
>>> May I ask you to add an explanation to the commit message why?
>>> If you do that you can add "Reviewed-by: Daniel Kiper <address@hidden>".
>>>
>>
>> The reason why we had this patch is that a following patch was using this
>> %X format specifier to print IP addresses as uppercase hexadecimal.
> 
> OK but then I would like to see patch using "%X" immediately following this 
> one.
> So, both have to be posted together.
>
> And please add similar explanation as you did above to the commit message.

Ok, I'll re-post with a better commit message as a series when posting the
other patch that's using the %X specifier then.

> 
>> But as Vladimir suggested, we could just use %x and grub_toupper(). So If
>> adding an %X format specifier seems unnecessary and would only make grub
>> core bigger, then I'll just change the other patch and drop this one.
> 
> TBH I prefer to have grub_printf() behavior similar to regular printf().
> And I do not think that this change will increase core image size much.
>

Yes, agreed.
 
> Daniel
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat