Ruiyu, I personally do not have a preference whether it has EFIAPI or not. But the declaration must be consistent with the implementation. When this function is not declared correctly, then the shell "cp" command does not work on GCC DEBUG builds. EFIAPI does carry weight in GCC and even CLANG/LLVM builds so consistency is important.
Does this have to remain an internal function? We could avoid this issue entirely if it were in a public header. It seems quite unusual for the EDK2 source to refer to a function in this manner. Regards, Thomas Palmer "I have only made this letter longer because I have not had the time to make it shorter" - Blaise Pascal -----Original Message----- From: Ni, Ruiyu [mailto:ruiyu...@intel.com] Sent: Monday, August 7, 2017 12:57 AM To: Palmer, Thomas <thomas.pal...@hpe.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kin...@intel.com> Subject: RE: [edk2] [PATCH 05/11] ShellPkg/UefiShellLevel2CommandsLib: Remove unnecessary EFIAPI Because an internal function doesn't need to have EFIAPI prefix. I think a proper fix is to change both BaseLib and UefiShellLevel2CommandsLib to add prefix to the function name. e.g.: ShellLevel2CommandsLibCharToUpper, BaseLibCharToUpper. And I am also surprised that BaseLib's version also has the EFIAPI prefix. ---BaseLib/String.c--- CHAR16 EFIAPI InternalCharToUpper ( IN CHAR16 Char ) Copying Mike for comments. Thanks/Ray > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Palmer, Thomas > Sent: Friday, August 4, 2017 3:07 AM > To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH 05/11] ShellPkg/UefiShellLevel2CommandsLib: > Remove unnecessary EFIAPI > > Ruiyu, > Sorry for replying to old patch, I was cleaning out some old > sandboxes when I stumbled onto this issue. > > Why is EFIAPI removed from InternalCharToUpper in > UefiShellLevel2CommandsLib.c? It is present in both BaseLibInternals.h and > String.c. Without EFIAPI, GCC builds of this function can fail > > > Regards, > > Thomas Palmer > > "I have only made this letter longer because I have not had the time > to make it shorter" - Blaise Pascal > > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Ruiyu Ni > Sent: Friday, September 30, 2016 3:18 AM > To: edk2-devel@lists.01.org > Cc: Ruiyu Ni <ruiyu...@intel.com>; Jaben Carsey > <jaben.car...@intel.com>; Chen A Chen <chen.a.c...@intel.com> > Subject: [edk2] [PATCH 05/11] ShellPkg/UefiShellLevel2CommandsLib: > Remove unnecessary EFIAPI > > From: Ruiyu Ni <ruiyu...@intel.com> > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Chen A Chen <chen.a.c...@intel.com> > Cc: Jaben Carsey <jaben.car...@intel.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > --- > ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c | 4 ---- > ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c | 2 -- > ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c | 5 ----- > ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c | 12 > ------------ > ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c | 7 ------- > ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c | 3 --- > ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c | 3 --- > ShellPkg/Library/UefiShellLevel2CommandsLib/Set.c | 1 - > ShellPkg/Library/UefiShellLevel2CommandsLib/TimeDate.c | 4 ---- > .../UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c | 5 ----- > .../UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h | 6 ------ > ShellPkg/Library/UefiShellLevel2CommandsLib/Vol.c | 1 - > 12 files changed, 53 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > index eb1f3b6..b8f6d31 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > @@ -35,7 +35,6 @@ > @retval SHELL_OUT_OF_RESOURCES a memory allocation failed > **/ > SHELL_STATUS > -EFIAPI > ValidateAndCopyFiles( > IN CONST EFI_SHELL_FILE_INFO *FileList, > IN CONST CHAR16 *DestDir, > @@ -58,7 +57,6 @@ ValidateAndCopyFiles( > @retval SHELL_SUCCESS The source file was copied to the destination > **/ > SHELL_STATUS > -EFIAPI > CopySingleFile( > IN CONST CHAR16 *Source, > IN CONST CHAR16 *Dest, > @@ -291,7 +289,6 @@ CopySingleFile( > @retval SHELL_OUT_OF_RESOURCES a memory allocation failed > **/ > SHELL_STATUS > -EFIAPI > ValidateAndCopyFiles( > IN CONST EFI_SHELL_FILE_INFO *FileList, > IN CONST CHAR16 *DestDir, > @@ -576,7 +573,6 @@ ValidateAndCopyFiles( > @retval SHELL_SUCCESS The operation was successful. > **/ > SHELL_STATUS > -EFIAPI > ProcessValidateAndCopyFiles( > IN EFI_SHELL_FILE_INFO *FileList, > IN CONST CHAR16 *DestDir, > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > index ff7c818..322d510 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > @@ -28,7 +28,6 @@ > > **/ > EFI_STATUS > -EFIAPI > ConnectAllEfi ( > VOID > ) > @@ -74,7 +73,6 @@ ConnectAllEfi ( > @retval EFI_OUT_OF_RESOURCES there was insufficient memory **/ > EFI_STATUS -EFIAPI LoadDriver( > IN CONST CHAR16 *FileName, > IN CONST BOOLEAN Connect > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c > index 9b4c452..52ae18f 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c > @@ -22,7 +22,6 @@ > @param[in] TheList a list of files from the volume. > **/ > EFI_STATUS > -EFIAPI > PrintSfoVolumeInfoTableEntry( > IN CONST EFI_SHELL_FILE_INFO *TheList > ) > @@ -152,7 +151,6 @@ PrintSfoVolumeInfoTableEntry( > > **/ > VOID > -EFIAPI > PrintFileInformation( > IN CONST BOOLEAN Sfo, > IN CONST EFI_SHELL_FILE_INFO *TheNode, @@ -263,7 +261,6 @@ > PrintFileInformation( > @param[in] Path String with starting path. > **/ > VOID > -EFIAPI > PrintNonSfoHeader( > IN CONST CHAR16 *Path > ) > @@ -300,7 +297,6 @@ PrintNonSfoHeader( > @param[in] Dirs The number of directories. > **/ > VOID > -EFIAPI > PrintNonSfoFooter( > IN UINT64 Files, > IN UINT64 Size, > @@ -339,7 +335,6 @@ PrintNonSfoFooter( > @retval SHELL_SUCCESS the printing was sucessful. > **/ > SHELL_STATUS > -EFIAPI > PrintLsOutput( > IN CONST BOOLEAN Rec, > IN CONST UINT64 Attribs, > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > index 035aff1..20eb528 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > @@ -34,7 +34,6 @@ > @retval FALSE String has at least one other character. > **/ > BOOLEAN > -EFIAPI > IsNumberLetterOnly( > IN CONST CHAR16 *String, > IN CONST UINTN Len > @@ -65,7 +64,6 @@ IsNumberLetterOnly( > items (";" normally). > **/ > BOOLEAN > -EFIAPI > SearchList( > IN CONST CHAR16 *List, > IN CONST CHAR16 *MetaTarget, > @@ -133,7 +131,6 @@ SearchList( > @retval STR_MAP_MEDIA_FLOPPY The media is a floppy drive. > **/ > CHAR16* > -EFIAPI > GetDeviceMediaType ( > IN EFI_DEVICE_PATH_PROTOCOL *DevicePath > ) > @@ -179,7 +176,6 @@ GetDeviceMediaType ( > @retval FALSE The handle does not have removable > storage. > **/ > BOOLEAN > -EFIAPI > IsRemoveableDevice ( > IN EFI_DEVICE_PATH_PROTOCOL *DevicePath > ) > @@ -216,7 +212,6 @@ IsRemoveableDevice ( > @retval FALSE The map should not be displayed. > **/ > BOOLEAN > -EFIAPI > MappingListHasType( > IN CONST CHAR16 *MapList, > IN CONST CHAR16 *Specific, > @@ -287,7 +282,6 @@ MappingListHasType( > @retval EFI_SUCCESS The mapping was displayed. > **/ > EFI_STATUS > -EFIAPI > PerformSingleMappingDisplay( > IN CONST BOOLEAN Verbose, > IN CONST BOOLEAN Consist, > @@ -461,7 +455,6 @@ PerformSingleMappingDisplay( > @retval EFI_NOT_FOUND Name was not a map on Handle. > **/ > EFI_STATUS > -EFIAPI > PerformSingleMappingDelete( > IN CONST CHAR16 *Specific, > IN CONST EFI_HANDLE Handle > @@ -512,7 +505,6 @@ CONST CHAR16 AnyF[] = L"F*"; > > **/ > SHELL_STATUS > -EFIAPI > PerformMappingDisplay( > IN CONST BOOLEAN Verbose, > IN CONST BOOLEAN Consist, > @@ -690,7 +682,6 @@ PerformMappingDisplay( > @sa PerformMappingDisplay > **/ > SHELL_STATUS > -EFIAPI > PerformMappingDisplay2( > IN CONST BOOLEAN Verbose, > IN CONST BOOLEAN Consist, > @@ -743,7 +734,6 @@ PerformMappingDisplay2( > @retval EFI_NOT_FOUND Specific could not be found. > **/ > EFI_STATUS > -EFIAPI > PerformMappingDelete( > IN CONST CHAR16 *Specific > ) > @@ -874,7 +864,6 @@ PerformMappingDelete( > > **/ > SHELL_STATUS > -EFIAPI > AddMappingFromMapping( > IN CONST CHAR16 *Map, > IN CONST CHAR16 *SName > @@ -931,7 +920,6 @@ AddMappingFromMapping( > > **/ > SHELL_STATUS > -EFIAPI > AddMappingFromHandle( > IN CONST EFI_HANDLE Handle, > IN CONST CHAR16 *SName > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > index f93772c..efaaeb2 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > @@ -26,7 +26,6 @@ > @retval FALSE The move is within a file system. > **/ > BOOLEAN > -EFIAPI > IsBetweenFileSystem( > IN CONST CHAR16 *FullName, > IN CONST CHAR16 *Cwd, > @@ -79,7 +78,6 @@ IsBetweenFileSystem( > @retval FALSE The move is not > **/ > BOOLEAN > -EFIAPI > IsValidMove( > IN CONST CHAR16 *SourcePath, > IN CONST CHAR16 *Cwd, > @@ -161,7 +159,6 @@ IsValidMove( > @retval SHELL_SUCCESS The operation was sucessful. > **/ > SHELL_STATUS > -EFIAPI > GetDestinationLocation( > IN CONST CHAR16 *DestParameter, > IN OUT CHAR16 **DestPathPointer, > @@ -286,7 +283,6 @@ GetDestinationLocation( > @retval SHELL_SUCCESS The source file was moved to the destination. > **/ > EFI_STATUS > -EFIAPI > MoveBetweenFileSystems( > IN EFI_SHELL_FILE_INFO *Node, > IN CONST CHAR16 *DestPath, > @@ -334,7 +330,6 @@ MoveBetweenFileSystems( > @retval SHELL_OUT_OF_RESOURCES a memory allocation failed > **/ > EFI_STATUS > -EFIAPI > CreateFullDestPath( > IN CONST CHAR16 **DestPath, > OUT CHAR16 **FullDestPath, > @@ -373,7 +368,6 @@ CreateFullDestPath( > @retval SHELL_OUT_OF_RESOURCES A memory allocation failed. > **/ > EFI_STATUS > -EFIAPI > MoveWithinFileSystems( > IN EFI_SHELL_FILE_INFO *Node, > IN CHAR16 *DestPath, > @@ -454,7 +448,6 @@ MoveWithinFileSystems( > @retval SHELL_OUT_OF_RESOURCES a memory allocation failed > **/ > SHELL_STATUS > -EFIAPI > ValidateAndMoveFiles( > IN EFI_SHELL_FILE_INFO *FileList, > OUT VOID **Resp, > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c > index 12fe877..4b1973a 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Parse.c > @@ -78,7 +78,6 @@ IsStdInDataAvailable ( > Size was updated to the minimum space > required. > **/ > EFI_STATUS > -EFIAPI > ShellFileHandleReadStdInLine( > IN SHELL_FILE_HANDLE Handle, > IN OUT CHAR16 *Buffer, > @@ -160,7 +159,6 @@ ShellFileHandleReadStdInLine( > @sa ShellFileHandleReadLine > **/ > CHAR16* > -EFIAPI > ParseReturnStdInLine ( > IN SHELL_FILE_HANDLE Handle > ) > @@ -249,7 +247,6 @@ HandleStringWithEscapeCharForParse ( > @retval SHELL_SUCCESS The operation was successful. > **/ > SHELL_STATUS > -EFIAPI > PerformParsing( > IN CONST CHAR16 *FileName, > IN CONST CHAR16 *TableName, > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c > index 0b23fba..618610d 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c > @@ -29,7 +29,6 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = { > @retval FALSE The directory has at least 1 file or directory in it. > **/ > BOOLEAN > -EFIAPI > IsDirectoryEmpty ( > IN EFI_HANDLE FileHandle > ) > @@ -66,7 +65,6 @@ IsDirectoryEmpty ( > @retval SHELL_DEVICE_ERROR A device error occured reading this Node. > **/ > SHELL_STATUS > -EFIAPI > CascadeDelete( > IN EFI_SHELL_FILE_INFO *Node, > IN CONST BOOLEAN Quiet > @@ -195,7 +193,6 @@ CascadeDelete( > @param[in] Package RESERVED. Not used. > **/ > BOOLEAN > -EFIAPI > IsValidDeleteTarget( > IN CONST EFI_SHELL_FILE_INFO *List, > IN CONST EFI_SHELL_FILE_INFO *Node, diff --git > a/ShellPkg/Library/UefiShellLevel2CommandsLib/Set.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Set.c > index d5e6a08..7ca1942 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Set.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Set.c > @@ -24,7 +24,6 @@ > @return any return code from GetNextVariableName except > EFI_NOT_FOUND **/ SHELL_STATUS -EFIAPI PrintAllShellEnvVars( > VOID > ) > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/TimeDate.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/TimeDate.c > index 533519d..3ebc72a 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/TimeDate.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/TimeDate.c > @@ -28,7 +28,6 @@ > @retval FALSE String is invalid. > **/ > BOOLEAN > -EFIAPI > InternalIsTimeLikeString ( > IN CONST CHAR16 *String, > IN CONST CHAR16 Char, > @@ -87,7 +86,6 @@ InternalIsTimeLikeString ( > @retval SHELL_SUCCESS The operation was successful. > **/ > SHELL_STATUS > -EFIAPI > CheckAndSetDate ( > IN CONST CHAR16 *DateString > ) > @@ -301,7 +299,6 @@ STATIC CONST SHELL_PARAM_ITEM TimeParamList3[] = { > @retval SHELL_SUCCESS The operation was successful. > **/ > SHELL_STATUS > -EFIAPI > CheckAndSetTime ( > IN CONST CHAR16 *TimeString, > IN CONST INT16 Tz, > @@ -701,7 +698,6 @@ STATIC CONST SHELL_PARAM_ITEM TimeZoneParamList3[] > = { > @retval SHELL_SUCCESS The operation was successful. > **/ > SHELL_STATUS > -EFIAPI > CheckAndSetTimeZone ( > IN CONST CHAR16 *TimeZoneString > ) > diff --git > a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands > Lib.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands > Lib.c > index 0dafb19..1491ee9 100644 > --- > a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands > Lib.c > +++ > b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command > +++ sLib.c > @@ -169,7 +169,6 @@ ShellLevel2CommandsLibDestructor ( > @retval other An allocated pointer to a fuly qualified path. > **/ > CHAR16* > -EFIAPI > GetFullyQualifiedPath( > IN CONST CHAR16* Path > ) > @@ -216,7 +215,6 @@ GetFullyQualifiedPath( > @retval EFI_SUCCESS The operation was successful. > **/ > EFI_STATUS > -EFIAPI > VerifyIntermediateDirectories ( > IN CONST CHAR16 *Path > ) > @@ -270,7 +268,6 @@ VerifyIntermediateDirectories ( > @return Char as an upper case character. > **/ > CHAR16 > -EFIAPI > InternalCharToUpper ( > IN CONST CHAR16 Char > ); > @@ -286,7 +283,6 @@ InternalCharToUpper ( > @return The location in Source where there is a difference. > **/ > CONST CHAR16* > -EFIAPI > StrniCmp( > IN CONST CHAR16 *Source, > IN CONST CHAR16 *Target, > @@ -322,7 +318,6 @@ StrniCmp( > @retval EFI_SUCCESS The operation was successful. > **/ > EFI_STATUS > -EFIAPI > ShellLevel2StripQuotes ( > IN CONST CHAR16 *OriginalString, > OUT CHAR16 **CleanString > diff --git > a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands > Lib.h > b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands > Lib.h > index 634515e..19e46a1 100644 > --- > a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands > Lib.h > +++ > b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command > +++ sLib.h > @@ -263,7 +263,6 @@ ShellCommandRunMv ( > @retval other pointer to a fuly qualified path. > **/ > CHAR16* > -EFIAPI > GetFullyQualifiedPath( > IN CONST CHAR16* Path > ); > @@ -276,7 +275,6 @@ GetFullyQualifiedPath( > @retval EFI_SUCCESS The operation was successful. > **/ > EFI_STATUS > -EFIAPI > VerifyIntermediateDirectories ( > IN CONST CHAR16 *Path > ); > @@ -292,7 +290,6 @@ VerifyIntermediateDirectories ( > @return non-zero if the strings are different. > **/ > CONST CHAR16* > -EFIAPI > StrniCmp( > IN CONST CHAR16 *Source, > IN CONST CHAR16 *Target, > @@ -310,7 +307,6 @@ StrniCmp( > @retval EFI_SUCCESS The operation was successful. > **/ > EFI_STATUS > -EFIAPI > ShellLevel2StripQuotes ( > IN CONST CHAR16 *OriginalString, > OUT CHAR16 **CleanString > @@ -343,7 +339,6 @@ ShellCommandRunVol ( > @retval SHELL_SUCCESS The source file was copied to the destination > **/ > SHELL_STATUS > -EFIAPI > CopySingleFile( > IN CONST CHAR16 *Source, > IN CONST CHAR16 *Dest, > @@ -364,7 +359,6 @@ CopySingleFile( > @retval SHELL_DEVICE_ERROR A device error occured reading this Node. > **/ > SHELL_STATUS > -EFIAPI > CascadeDelete( > IN EFI_SHELL_FILE_INFO *Node, > IN CONST BOOLEAN Quiet > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Vol.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Vol.c > index a6f0296..f911c7e 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Vol.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Vol.c > @@ -27,7 +27,6 @@ > @retval SHELL_SUCCESS The operation was sucessful. > **/ > SHELL_STATUS > -EFIAPI > HandleVol( > IN CONST CHAR16 *Path, > IN CONST BOOLEAN Delete, > -- > 2.9.0.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel