All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>
To: The development of GNU GRUB <grub-devel@gnu.org>
Subject: Re: Bug#584474: FTBFS: usbms.c:315: error: format ‘%02x’ expects type ‘unsigned int’
Date: Sat, 05 Jun 2010 01:01:31 +0200	[thread overview]
Message-ID: <4C0985CB.60506@gmail.com> (raw)
In-Reply-To: <20100604224002.GC21862@riva.ucam.org>

[-- Attachment #1: Type: text/plain, Size: 3130 bytes --]

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

      reply	other threads:[~2010-06-04 23:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-03 19:27 Bug#584474: FTBFS: usbms.c:315: error: format ‘%02x’ expects type ‘unsigned int’ sean finney
2010-06-04 12:09 ` Colin Watson
2010-06-04 22:17   ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-06-04 22:40     ` Colin Watson
2010-06-04 23:01       ` Vladimir 'φ-coder/phcoder' Serbinenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C0985CB.60506@gmail.com \
    --to=phcoder@gmail.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.