Re: [PATCH v3 1/2] Add %X to grub_vsnprintf_real and friends

[Steve McIntyre]

Apologies for the delayed review - lots of travel at the moment... :-/

On Sat, Mar 23, 2019 at 01:42:08PM +0000, Colin Watson wrote:
>This is needed for UEFI Boot* variables, which the standard says are
>named using upper-case hexadecimal.
>
>Signed-off-by: Colin Watson <address@hidden>
>---
> grub-core/kern/misc.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>index 3b633d51f..18cad5803 100644
>--- a/grub-core/kern/misc.c
>+++ b/grub-core/kern/misc.c
>@@ -588,7 +588,7 @@ grub_divmod64 (grub_uint64_t n, grub_uint64_t d, 
>grub_uint64_t *r)
> static inline char *
> grub_lltoa (char *str, int c, unsigned long long n)
> {
>-  unsigned base = (c == 'x') ? 16 : 10;
>+  unsigned base = (c == 'x' || c == 'X') ? 16 : 10;
>   char *p;
> 
>   if ((long long) n < 0 && c == 'd')
>@@ -603,7 +603,7 @@ grub_lltoa (char *str, int c, unsigned long long n)
>     do
>       {
>       unsigned d = (unsigned) (n & 0xf);
>-      *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
>+      *p++ = (d > 9) ? d + ((c == 'x') ? 'a' : 'A') - 10 : d + '0';

Yay, nested ternaries! Hardly the most readable code, but meh.

>       }
>     while (n >>= 4);
>   else
>@@ -676,6 +676,7 @@ parse_printf_args (const char *fmt0, struct printf_args 
>*args,
>       {
>       case 'p':
>       case 'x':
>+      case 'X':
>       case 'u':
>       case 'd':
>       case 'c':
>@@ -762,6 +763,7 @@ parse_printf_args (const char *fmt0, struct printf_args 
>*args,
>       switch (c)
>       {
>       case 'x':
>+      case 'X':
>       case 'u':
>         args->ptr[curn].type = UNSIGNED_INT + longfmt;
>         break;
>@@ -900,6 +902,7 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const 
>char *fmt0,
>         c = 'x';
>         /* Fall through. */
>       case 'x':
>+      case 'X':
>       case 'u':
>       case 'd':
>         {

Reviewed-by: Steve McIntyre <address@hidden>

-- 
Steve McIntyre, Cambridge, UK.                                address@hidden
"I suspect most samba developers are already technically insane... Of
 course, since many of them are Australians, you can't tell." -- Linus Torvalds