On 23/01/2024 17:10, Laszlo Ersek wrote:
Other than that, the patch looks great to me; I'd only propose one
(superficial) improvement:
rather than include <Library/ArmLib.h>, scavenge
#ifdef MDE_CPU_ARM
#include <Chipset/ArmV7.h>
#elif defined (MDE_CPU_AARCH64)
#include <Chipset/AArch64.h>
#endif
from it.
Reasons:
(a) Those are the headers that directly provide the macros we need, no
need to include the rest of ArmLib.h. (Listing ArmPkg/ArmPkg.dec in the
Packages section of the INF file will make these more direct #include
directives work, too.)
(b) Including <Library/ArmLib.h> kind of de-synchronizes the #include
directives in the C source from the [LibraryClasses] section in the INF
file. Generally there should be a 1-to-1 match -- we should include the
declarations of variables and functions for exactly those libraries that
we link against. There are two exceptions (that I can think of at once):
when we only want macros from a lib class header, or when we include a
lib class header because we are implementing an instance for that lib
class (i.e., we're providing, not consuming, the definitions for the
header-declared variables and functions). In this case, neither seems to
apply, this is not an ArmLib instance (= implementation), and the macros
we need don't actually come from ArmLib.h.
Acked-by: Laszlo Ersek <ler...@redhat.com>
Will do, thanks!
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114225): https://edk2.groups.io/g/devel/message/114225
Mute This Topic: https://groups.io/mt/103911611/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
