Re: Bug#584474: FTBFS: usbms.c:315: error: form at ‘%02x’ expects type ‘unsigned int ’

[Colin Watson]

On Thu, Jun 03, 2010 at 09:27:38PM +0200, sean finney wrote:
> while investigated an unrelated bug, i noticed that the latest snapshot
> uploaded to debian fails with the following error:
> 
> gcc-4.4 -Idisk -I/home/seanius/grub2-1.98+20100602/disk 
> -I/home/seanius/grub2-1.98+20100602/include -I. -I./include -Wall -W  
> -ffreestanding  -Os -DGRUB_MACHINE_EMU=1 -DMACHINE=X86_64_EMU -Wall -W 
> -Wshadow -Wpointer-arith -Wmissing-prototypes                -Wundef 
> -Wstrict-prototypes -g -fno-dwarf2-cfi-asm -m64 -fno-stack-protector 
> -mno-stack-arg-probe -Werror -DGRUB_TARGET_NO_MODULES=1 
> -DUSE_ASCII_FAILBACK=1  -DGRUB_FILE=\"disk/usbms.c\" -MD -c -o 
> usbms_mod-disk_usbms.o /home/seanius/grub2-1.98+20100602/disk/usbms.c
> cc1: warnings being treated as errors
> /home/seanius/grub2-1.98+20100602/disk/usbms.c: In function 
> ‘grub_usbms_transfer’:
> /home/seanius/grub2-1.98+20100602/disk/usbms.c:315: error: format ‘%02x’ 
> expects type ‘unsigned int’, but argument 5 has type ‘grub_size_t’
> /home/seanius/grub2-1.98+20100602/disk/usbms.c:333: error: format ‘%02x’ 
> expects type ‘unsigned int’, but argument 5 has type ‘grub_size_t’
> make[1]: *** [usbms_mod-disk_usbms.o] Error 1
> 
> the two lines in question are both debug printf type statements.
> therefore, instead of doing something complicated to get the format
> string to match with the size of grub_size_t (which i'm guessing could
> vary based on the platform), i have created a patch that simply casts the
> parameter in question as unsigned to match the format string instead (which
> only shows two digits anyway).

As Vladimir pointed out on IRC: no, that's a minimum field width, not a
precision.  It shows *at least* two digits.

I think I'd prefer to add a grub_printf length modifier to print
grub_size_t values.  'z' is standard for size_t, so it seems reasonable
to use that, it can be done in very little code, and it saves on ugly
and inaccurate casts.  Vladimir, what do you think?

(If this is unacceptable for whatever reason, then the proper fix would
be to cast to (unsigned long long) and to use %02llx - but I prefer
having an accurate length modifier, really.)

2010-06-04  Colin Watson  <address@hidden>

        * kern/misc.c (grub_vsnprintf_real): Support 'z' length
        modifier, for grub_size_t.
        * disk/usbms.c (grub_usbms_transfer): Use it.

=== modified file 'disk/usbms.c'
--- disk/usbms.c        2010-06-01 00:10:19 +0000
+++ disk/usbms.c        2010-06-04 12:04:11 +0000
@@ -312,7 +312,7 @@ grub_usbms_transfer (struct grub_scsi *s
       grub_dprintf ("usb", "buf:\n");
       if (size <= 64)
         for (i=0; i<size; i++)
-          grub_dprintf ("usb", "0x%02x: 0x%02x\n", i, buf[i]);
+          grub_dprintf ("usb", "0x%02zx: 0x%02x\n", i, buf[i]);
       else
           grub_dprintf ("usb", "Too much data for debug print...\n");
     }
@@ -330,7 +330,7 @@ grub_usbms_transfer (struct grub_scsi *s
       /* Debug print of sent data. */
       if (size <= 256)
         for (i=0; i<size; i++)
-          grub_dprintf ("usb", "0x%02x: 0x%02x\n", i, buf[i]);
+          grub_dprintf ("usb", "0x%02zx: 0x%02x\n", i, buf[i]);
       else
           grub_dprintf ("usb", "Too much data for debug print...\n");
     }

=== modified file 'kern/misc.c'
--- kern/misc.c 2010-05-28 13:48:45 +0000
+++ kern/misc.c 2010-06-04 12:04:11 +0000
@@ -688,6 +688,7 @@ grub_vsnprintf_real (char *str, grub_siz
          int n;
          int longfmt = 0;
          int longlongfmt = 0;
+         int sizetfmt = 0;
          int unsig = 0;
 
          if (*fmt && *fmt =='-')
@@ -740,6 +741,11 @@ grub_vsnprintf_real (char *str, grub_siz
                  c = *fmt++;
                }
            }
+         else if (c == 'z')
+           {
+             sizetfmt = 1;
+             c = *fmt++;
+           }
 
          switch (c)
            {
@@ -770,6 +776,11 @@ grub_vsnprintf_real (char *str, grub_siz
                  long l = va_arg (args, long);
                  grub_lltoa (tmp, c, l);
                }
+             else if (sizetfmt)
+               {
+                 grub_size_t sz = va_arg (args, grub_size_t);
+                 grub_lltoa (tmp, c, sz);
+               }
              else if (unsig)
                {
                  unsigned u = va_arg (args, unsigned);

Thanks,

-- 
Colin Watson                                       address@hidden