Re: [PATCH] Check for the appropriate condition in types.h

[Pavel Roskin <proski@gnu.org>]

On Thu, 2009-07-23 at 20:11 +0200, Javier Martín wrote:
> El jue, 23-07-2009 a las 17:40 +0200, Vladimir 'phcoder' Serbinenko
> escribió:
> > Actualy I should have used ALIGN_UP here
> It's just a random sample of code, brought up by a search for (long).
> I'm pretty sure more examples abound.
> 
> > > Regarding the ChangeLog entry, I always have problems with them. What
> > > about this one?:
> > > 2009-07-23 Javier Martin <lordhabbit@gmail.com>
> > >        * include/grub/types.h (GRUB_CPU_SIZEOF_VOID_P): substitute for
> > This would suggest that you modify GRUB_CPU_SIZEOF_VOID_P macro which
> > isn't the case. I would prefer something like:
> > [GRUB_CPU_SIZEOF_VOID_P == 8]: Changed to ...
> > [GRUB_CPU_SIZEOF_LONG == 8]: ... this.
> Ok, let's adopt this form instead. The proposed ChangeLog would now be:

From the GNU Coding Standards:

"C programs often contain compile-time `#if' conditionals.  Many changes
are conditional; sometimes you add a new definition which is entirely
contained in a conditional.  It is very useful to indicate in the
change log the conditions for which the change applies.  Our convention
for indicating conditional changes is to use square brackets around the
name of the condition."

It means that the square brackets are used if the changes only affect
the code under the condition specified in brackets.  This is not what is
happening here.

>           (UINT_TO_PTR): move outside wordsize conditionals
>           (PTR_TO_UINT): new macro

We should remove PTR_TO_UINT32 and PTR_TO_UINT64 with PTR_TO_UINT
everywhere.  I've checked that it doesn't introduce any warnings on any
platform.

There should be no need to combine a cast to a scalar type with a cast
to change the type precision.  By eliminating the second cast, we make
it possible for the compiler to catch precision loss.

-- 
Regards,
Pavel Roskin