Re: Bug#584474: FTBFS: usbms.c:315: erro r: format ‘%02x’ expects type ‘unsigned int’

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 06/04/2010 02:09 PM, Colin Watson wrote:
> 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?
>
>   
grub_size_t is always of the same length as long. Perhaps we should
define it as unsigned long, use "%lx" and get rid of useless difference
on 32-bit and 64-bit platforms.
Are there any reasons not to do this way?
Alternatively %zx can be aliased to %lx in one code line.
> (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,
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature