[edk2-devel] [PATCH v2 3/3] MdeModulePkg: Use CopyMem instead of GUID assignment

[Leif Lindholm]

+Ard, Laszlo.

I think it would make sense to move it to MdeModulePkg (or MdePkg) and
rename it BaseCompilerIntrinsicsLib (it *is* a BASE library).

As I alluded in my reply to Ray - x86 also have this problem, but to a
lesser extent, and ended up creating library functions to call instead
of using plain C for certain operations.
Which was probably the right decision when it was restricted to a
very few corner cases.

/
    Leif

On Thu, Mar 12, 2020 at 13:24:30 +0000, Schaefer, Daniel (DualStudy) wrote:
> Hi Leif,
> 
> you're right. If I revert my commit and include
>   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> without making any changes to it, the build succeeds.
> 
> What do others think? (cc Michael, Pete, Andrew, Ard, who have made changes to this module)
> Is this a big hack or should we use it in RISC-V, too and move the module to MdeModulePkg?
> Why isn't this a problem on x86? Was it fine on Itanium?
> 
> - Daniel
> 
> ________________________________
> From: devel at edk2.groups.io<mailto:devel at edk2.groups.io> <devel at edk2.groups.io><mailto:devel at edk2.groups.io> on behalf of Leif Lindholm <leif at nuviainc.com><mailto:leif at nuviainc.com>
> Sent: Thursday, March 12, 2020 11:55
> To: devel at edk2.groups.io<mailto:devel at edk2.groups.io> <devel at edk2.groups.io><mailto:devel at edk2.groups.io>; Schaefer, Daniel (DualStudy) <daniel.schaefer at hpe.com><mailto:daniel.schaefer at hpe.com>
> Cc: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com><mailto:abner.chang at hpe.com>; Chen, Gilbert <gilbert.chen at hpe.com><mailto:gilbert.chen at hpe.com>; Dandan Bi <dandan.bi at intel.com><mailto:dandan.bi at intel.com>; Eric Dong <eric.dong at intel.com><mailto:eric.dong at intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 3/3] MdeModulePkg: Use CopyMem instead of GUID assignment
> 
> Hi Daniel,
> 
> There is nothing wrong with this patch that just went in (and I should
> have called out sooner if I wanted to stop it), but I think a better
> solution is to implement a RISC-V variant of
> ArmPkg/Library/CompilerIntrinsicsLib/.
> 
> It is perfectly valid for the compiler to generate memcpy calls in
> response to struct operations that are perfectly valid C.
> 
> In fact, we could consider moving the ArmPkg one over into
> MdeModulePkg. I have a feeling that including a
>   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> in your current build would be an alternative solution to your
> compilation error.
> 
> /
>     Leif
> 
> On Mon, Mar 02, 2020 at 11:32:38 +0100, Daniel Schaefer wrote:
> > GCC translates a simple assignment to memcpy, which EDKII doesn't provide.
> > See: https://www.mail-archive.com/edk2-devel@lists.01.org/msg11928.html
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2547
> >
> > Signed-off-by: Daniel Schaefer <daniel.schaefer at hpe.com><mailto:daniel.schaefer at hpe.com>
> > Cc: Abner Chang <abner.chang at hpe.com><mailto:abner.chang at hpe.com>
> > Cc: Gilbert Chen <gilbert.chen at hpe.com><mailto:gilbert.chen at hpe.com>
> > Cc: Leif Lindholm <leif at nuviainc.com><mailto:leif at nuviainc.com>
> > Cc: Dandan Bi <dandan.bi at intel.com><mailto:dandan.bi at intel.com>
> > Cc: Eric Dong <eric.dong at intel.com><mailto:eric.dong at intel.com>
> > ---
> >
> > Notes:
> >     v2:
> >       - Use CopyMem instead of CopyGuid [Dandan]
> >
> >  MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> > index 5cc527679a78..0540e6fa8a44 100644
> > --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> > +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> > @@ -619,7 +619,7 @@ CreateDeviceManagerForm(
> >          TokenHelp = HiiSetString (HiiHandle, 0, String, NULL);
> >          FreePool (String);
> >
> > -        FormSetGuid = ((EFI_IFR_FORM_SET *)Ptr)->Guid;
> > +        CopyMem (&FormSetGuid, &((EFI_IFR_FORM_SET *) Ptr)->Guid, sizeof (EFI_GUID));
> >
> >          //
> >          // Network device process
> > --
> > 2.25.0
> >
> >
> >
> >
> 
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55811): https://edk2.groups.io/g/devel/message/55811
Mute This Topic: https://groups.io/mt/71671270/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-