El jue, 23-07-2009 a las 10:37 -0400, Pavel Roskin escribió: > On Thu, 2009-07-23 at 13:10 +0200, Javier Martín wrote: > > Currently <grub/types.h> verifies that sizeof(void*)==sizeof(long) and > > then proceeds to define most fixed-length types based on sizeof(void*). > > While simplification seems a good idea on paper, it is always good > > practice to check specifically for what we want to know. In particular, > > the assumption that long can hold a pointer will go down when (if) > > mingw64 support is merged, because Win64 follows the LLP64 model instead > > of the LP64 *nix model. > > > > Given that currently both values are, by precondition, the same, this > > change is guaranteed not to create any problems, while it might avoid > > them in the future. > > The patch is good. Please include the ChangeLog entry. > Thanks. I have though of an additional change: if it seems acceptable, apply the "new" version of the patch that I attach with this message. If it is not, just use the old version in the original post and I'll send the new change as another patch.
The additional change is the refactoring of the UINT_TO_PTR macro. Given
that we have a grub_addr_t type fulfilling a role similar to the
standard uintptr_t (an integral type "long enough to hold a pointer and
back"), there is no need for the conditional definition of UINT_TO_PTR
as either casting to a 32 or 64-bit type: grub_addr_t is defined exactly
like that.
Furthermore, I have added a PTR_TO_UINT macro as a means to perform
safer* "hard" pointer arithmetic that frequently comes up in low-level
code without having to think of the types to cast to and back. This way,
this random code: (in acpi.c)
target = (grub_uint8_t *) ((((long) target - 1) | 0xf) + 1);
Would become:
target = UINT_TO_PTR(((PTR_TO_UINT(target) - 1) | 0xf) + 1);
Which is safe* as long as grub_addr_t is properly defined. Thus, this
new macro will allow coders to gather scattered typing decisions and
centralize them to types.h. The process would be just: PTR_TO_UINT ->
perform weird arithmetic -> UINT_TO_PTR.
* safer in the sense of not risking any truncations or "undefined
behavior" if the addr_t types change, of course, not for the actual
arithmetic being performed
Regarding the ChangeLog entry, I always have problems with them. What
about this one?:
2009-07-23 Javier Martin <[email protected]>
* include/grub/types.h (GRUB_CPU_SIZEOF_VOID_P): substitute for
GRUB_CPU_SIZEOF_LONG where appropriate
(UINT_TO_PTR): move outside wordsize conditionals
(PTR_TO_UINT): new macro
--
-- Lazy, Oblivious, Recurrent Disaster -- Habbit
diff --git a/include/grub/types.h b/include/grub/types.h index 8e2ad15..72f1288 100644 --- a/include/grub/types.h +++ b/include/grub/types.h @@ -62,7 +62,7 @@ typedef signed char grub_int8_t; typedef short grub_int16_t; typedef int grub_int32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 typedef long grub_int64_t; #else typedef long long grub_int64_t; @@ -71,7 +71,7 @@ typedef long long grub_int64_t; typedef unsigned char grub_uint8_t; typedef unsigned short grub_uint16_t; typedef unsigned grub_uint32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 typedef unsigned long grub_uint64_t; #else typedef unsigned long long grub_uint64_t; @@ -100,7 +100,7 @@ typedef grub_uint32_t grub_size_t; typedef grub_int32_t grub_ssize_t; #endif -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 # define GRUB_ULONG_MAX 18446744073709551615UL # define GRUB_LONG_MAX 9223372036854775807L # define GRUB_LONG_MIN (-9223372036854775807L - 1) @@ -110,12 +110,12 @@ typedef grub_int32_t grub_ssize_t; # define GRUB_LONG_MIN (-2147483647L - 1) #endif +#define UINT_TO_PTR(x) ((void*)(grub_addr_t)(x)) +#define PTR_TO_UINT(x) ((grub_addr_t)(x)) #if GRUB_CPU_SIZEOF_VOID_P == 4 -#define UINT_TO_PTR(x) ((void*)(grub_uint32_t)(x)) #define PTR_TO_UINT64(x) ((grub_uint64_t)(grub_uint32_t)(x)) #define PTR_TO_UINT32(x) ((grub_uint32_t)(x)) #else -#define UINT_TO_PTR(x) ((void*)(grub_uint64_t)(x)) #define PTR_TO_UINT64(x) ((grub_uint64_t)(x)) #define PTR_TO_UINT32(x) ((grub_uint32_t)(grub_uint64_t)(x)) #endif
signature.asc
Description: Esto es una parte de mensaje firmado digitalmente
_______________________________________________ Grub-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/grub-devel
