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 [[email protected]] _______________________________________________ Grub-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/grub-devel
