efi.h: Fix truncation of constant value
PATCH 1/1 efi.h: Fix truncation of constant value
Starting with commit 867a6ac86dd8 ("efi: Add start-up library code"), sparse constantly complains about truncated constant value in efi.h:
include/efi.h:176:35: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
This can get quite noisy, preventing real issues to be noticed:
$ make defconfig *** Default configuration is based on 'sandbox_defconfig' $ make C=2 -j12 2>&1 | grep truncates | wc -l 441
After the patch is applied:
$ make C=2 -j12 2>&1 | grep truncates | wc -l 0 $ sparse --version v0.5.2
--- include/efi.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
include/efi.h +3 -3
@@ -172,11 +172,11 @@ enum {EFI_MEMORY_RP_SHIFT =13 ,/* read-protect */ EFI_MEMORY_XP_SHIFT =14 ,/* execute-protect */ EFI_MEMORY_RUNTIME_SHIFT = 63, /* range requires runtime mapping */ - -EFI_MEMORY_RUNTIME =1ULL <<EFI_MEMORY_RUNTIME_SHIFT , - EFI_MEM_DESC_VERSION = 1, }; +#define EFI_MEMORY_RUNTIME (1ULL << EFI_MEMORY_RUNTIME_SHIFT) +#define EFI_MEM_DESC_VERSION 1 +#define EFI_PAGE_SHIFT 12#define EFI_PAGE_SIZE (1UL << EFI_PAGE_SHIFT)#define EFI_PAGE_MASK (EFI_PAGE_SIZE - 1)
Signature · 1 line
2.18.0
PATCH 1/1 efi.h: Fix truncation of constant value
Quoted 29 lines · Eugeniu Rosca
Starting with commit
867a6ac86dd8("efi: Add start-up library code"), sparse constantly complains about truncated constant value inefi.h:
include/efi.h:176:35: warning: cast truncates bits from constant value (8000000000000000 becomes 0)This can get quite noisy, preventing real issues to be noticed:
$ make defconfig *** Default configuration is based on 'sandbox_defconfig' $ make C=2 -j12 2>&1 | grep truncates | wc -l 441After the patch is applied:
$ make C=2 -j12 2>&1 | grep truncates | wc -l 0 $ sparse --version v0.5.2Fixes 867a6ac86dd8 ("efi: Add start-up library code")Signed-off-by Eugeniu Rosca <[email protected]>--- include/efi.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/efi.h +0
-0
include/efi.h enum { · line 177
Quoted 9 lines
include/efi.h +1
-3@@ -172,11 +172,11 @@ enum {EFI_MEMORY_RP_SHIFT =13 ,/* read-protect */ EFI_MEMORY_XP_SHIFT =14 ,/* execute-protect */ EFI_MEMORY_RUNTIME_SHIFT = 63, /* range requires runtime mapping */ - -EFI_MEMORY_RUNTIME =1ULL <<EFI_MEMORY_RUNTIME_SHIFT , - EFI_MEM_DESC_VERSION = 1, }; +#define EFI_MEMORY_RUNTIME (1ULL << EFI_MEMORY_RUNTIME_SHIFT)
Hello Eugeniu,
thanks for pointing out the problem.
We should not look at this single value only. Please eliminate the whole enum and use #define for all values using the names and definitions provided in the UEFI 2.7 spec (i.e. without the _SHIFT suffix).
#define EFI_MEMORY_UC 0x0000000000000001 #define EFI_MEMORY_WC 0x0000000000000002 #define EFI_MEMORY_WT 0x0000000000000004 #define EFI_MEMORY_WB 0x0000000000000008 #define EFI_MEMORY_UCE 0x0000000000000010 #define EFI_MEMORY_WP 0x0000000000001000 #define EFI_MEMORY_RP 0x0000000000002000 #define EFI_MEMORY_XP 0x0000000000004000 #define EFI_MEMORY_NV 0x0000000000008000
#define EFI_MEMORY_MORE_RELIABLE 0x0000000000010000
#define EFI_MEMORY_RO 0x0000000000020000
#define EFI_MEMORY_RUNTIME 0x8000000000000000
I think Simon used the _SHIFT values to save a few bytes in mem_attr[] on 32bit architectures@the cost of more bytes in coding on 64bit architectures.
In cmd/efi.c we could use the ilog2() macro to get the shift values. But I would prefer to remove the shifting altogether and go for more readable code.
Some values are missing in mem_attr[] (in cmd/efi.c). These should be added.
Best regards
Heinrich
Quoted 6 lines
include/efi.h +2
-0+#define EFI_MEM_DESC_VERSION 1 +#define EFI_PAGE_SHIFT 12#define EFI_PAGE_SIZE (1UL << EFI_PAGE_SHIFT)#define EFI_PAGE_MASK (EFI_PAGE_SIZE - 1)
PATCH 1/1 efi.h: Fix truncation of constant value
Hi Heinrich,
Many thanks for quick and useful comments.
I've implemented your suggestions in v2. TIA for review.
Best regards,
Eugeniu.