grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC,PATCH] C99 format specifiers for fixed-length integer types


From: Pavel Roskin
Subject: Re: [RFC,PATCH] C99 format specifiers for fixed-length integer types
Date: Wed, 22 Jul 2009 22:31:48 -0400

On Thu, 2009-07-23 at 04:03 +0200, Javier Martín wrote:
> El mié, 22-07-2009 a las 21:08 -0400, Pavel Roskin escribió:
> > I doubt about "runtime weirdness".  gcc is good at catching such
> > problems at the compile time.
> Yes, in naked expressions. However, once we start using casts the C
> compiler tends to be quite silent about the whole nitty-gritty process.
> For example, the following code generates no warnings (with -ansi
> -pedantic -Wall -Wconversion):
> 
>     unsigned short a = (unsigned short) 123456789;
>     uint16_t b = (uint16_t) UINT32_C(123456789);
> 
> That is, casts shut the compiler up (without the casts gcc just shows a
> warning about the truncation being performed). The runtime value of both
> a and b is 0xcd15 in my x86_64 Linux box.

That's totally expected.  But it doesn't represent a full scenario when
something can go wrong when porting to another platform.

> > It would be a good idea to use standard modifiers if they were not so
> > hard on the eye.
> Well, I suggested GRUB_x names for the macros, where x is the C99
> standard name, but we could use them directly as PRIx64 and the like.
> About verbosity, though... Have you programmed in Java? Some appearances
> of GRUB_PRId32 are nothing compared to System.out.println or
> java.util.ArrayList<Integer>, yet I write the last two religiously.

We are not writing GRUB in Java.  I guess Java also offers way to
simplify code and to check its correctness that we don't have in C.

> > That's a biased example where the size of the arguments is obvious.  And
> > even then, it's hard to read.  And if you put GRUB_PRI32o instead of
> > GRUB_PRI16o there, the compiler won't tell you anything.
> Hey, I'm the one proposing the patch. Surely you wouldn't expect me to
> be unbiased ;)
> Nevertheless, the issue you pointed happens because variadic arguments
> are automatically promoted with the following rules [1][2] from the
> ancient days of C:
>  - Integral types narrower than "int" -> int
>  - Floating point types narrower than "double" -> double
> So, you could argue, we could do without at the very least PRI?8 (and,
> here in grub, PRI?16 too). However, given how obscure this "feature" is
> (see [1]), I'd go for keeping all of them: orthogonality means less
> surprises for the future coders.

That's OK.

> > The patch includes the easy part, namely adding the macros.  But I
> > doesn't think it would be so pretty with the ugly part, that is,
> > "fixing" every almost every *printf call.  It will be very hard to
> > review.
> Well, of course. This patch only adds the infrastructure, or else it
> would be too invasive. Once it is in, a small number of people can start
> checking most source files and replace the old specifiers when
> appropriate.

And that's what I don't want to see.

We sacrifice readability and go through massive code changes.  We only
gain potential fixes for ports to hypothetical platforms.

In my opinion, it's not worth the trouble.  I'm not going to spend any
more time on this.  If you convince any maintainers, fine.

-- 
Regards,
Pavel Roskin




reply via email to

[Prev in Thread] Current Thread [Next in Thread]