Hi Mark,
On Wed, 27 Nov 2024 at 06:23, Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Simon Glass sjg@chromium.org Date: Wed, 27 Nov 2024 06:07:05 -0700
Hi Heinrich,
On Tue, 26 Nov 2024 at 08:37, Simon Glass sjg@chromium.org wrote:
Hi Heinrich,
On Tue, 26 Nov 2024 at 02:44, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 25.11.24 21:44, Simon Glass wrote:
Rather than an integer, it is better to use the enum provided, when referring to an EFI memory-type. Update existing uses.
The C standard provides no definition of the size of the integer used to implement enums. It could use u8, u16, u32, or u64.
As EFI applications may be compiled using a different compiler we need to control the width used for passing parameters in the API interface.
We should have used u32 instead of int here. We could use a typedef for more verbosity though that is frowned upon in U-Boot.
Yes, the ext function is debatable. The rest seem OK to me, though, as they are internal functions, See also [1].
Hmm, but the spec uses enum, so arguably this patch is more correct?
Only as a way to define the constants. The actual interfaces that use the constants use UINT32, which would indeed be u32 in Linux land as Heinrich suggests.
So no, the patch isn't correct.
Just to add a bit more detail...
I see the spec says this: "Element of a standard ANSI C enum type declaration. Type INT32.or UINT32. ANSI C does not define the size of sign of an enum so they should never be used in structures. ANSI C integer promotion rules make INT32 or UINT32 interchangeable when passed as an argument to a function."
The compilers we use (at least gcc and clang) use int as the enum type, and int is 32 bits on 32/64-bit machines.
So it looks safe to me. But since the spec goes out of its way to mention this point, it seems best to keep the external interface using int and just use the enum within the implementation code. That's what I did on v2 onwards.
Best regards
Heinrich
Call the value 'mem_type' consistently. Fix up one instance of upper-case hex.
Fix up the calls in struct efi_boot_services so that they use the same enum, adding the missing parameter names and enum efi_allocate_type.
While we are here, rename the 'memory' parameter to 'memoryp' so that it is clear it is a return value.
Signed-off-by: Simon Glass sjg@chromium.org
include/efi_api.h | 6 ++++-- include/efi_loader.h | 28 ++++++++++++++------------- lib/efi_loader/efi_boottime.c | 13 +++++++------ lib/efi_loader/efi_device_path.c | 4 ++-- lib/efi_loader/efi_memory.c | 33 ++++++++++++++++---------------- 5 files changed, 45 insertions(+), 39 deletions(-)
Regards, SImon
[1] https://stackoverflow.com/questions/4879286/specifying-size-of-enum-type-in-...
Regards, Simon