On 06/05/2010 12:40 AM, Colin Watson wrote:
> 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.  
Of course, I've never suggested an elimination of grub_size_t only doing
like:
typedef unsigned long grub_size_t;
> 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.
>   
Ok. We can do the same for grub_addr_t too.
> 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?
>
>   
It can be added but currently GRUB_CPU_SIZEOF_* changes between TARGET_*
and HOST_* so if sizes mismatch either util or target part won't compile.
>> 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"
>   
It's a good idea. Since it will be something heavily used perhaps it's
better to make an exception to usual GRUB style and shorten it somehow?
But if we do e.g. PRIxG_SIZE we may end up with conflicts with other
headers in utils
> 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.
>
>   
Actualy it's not so much of a "target" as a "compiler-target" pair.
Different compilers pick different machine-available sizes on same
platform for the same type of int.
> Thanks,
>
>   


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