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

[Colin Watson]

On Sat, Jun 05, 2010 at 12:17:57AM +0200, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> On 06/04/2010 02:09 PM, Colin Watson wrote:
> > 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?

Good point; I hadn't spotted that.  Mostly clarity, I think.  If it
comes out as unsigned long anyway then it doesn't make any difference in
the machine representation, but the different definitions do seem useful
for clarity.  If we change that (I had to read over it a few times to
convince myself), then we should comment it very carefully.  A comment
near the #error if GRUB_CPU_SIZEOF_VOID_P != GRUB_CPU_SIZEOF_LONG to say
that we'll need to change the grub_size_t definition if we ever support
an architecture where void * isn't the same length as long would also be
good, I think.

There is no assertion that GRUB_TARGET_SIZEOF_VOID_P ==
GRUB_TARGET_SIZEOF_LONG, although it's currently true for all targets.
Should there be?

> Alternatively %zx can be aliased to %lx in one code line.

I do like being explicit about the length modifier but I understand that
kern/misc.c needs to be as small as possible, so I'm not going to be
precious about it.  How about a compromise along the lines of C99's
<inttypes.h> to reduce the probability of introducing errors when we add
new targets?  Following that (unfortunately ugly) pattern, we'd get
something like:

  #define PRIxGRUB_SIZE "lx"

If you think it's unlikely that grub_size_t will ever need to be
anything other than unsigned long even on future targets, then maybe we
don't need to worry about that.

Thanks,

-- 
Colin Watson                                       address@hidden