Ruiyu,

Thanks for letting me know. I agree with below changes and they look good to 
me. Please go ahead and fix them. Hopefully I'll  get a chance to review/test 
it.

Paulo

-----Original Message-----
From: Gao, Liming [mailto:liming....@intel.com] 
Sent: Monday, October 30, 2017 3:27 AM
To: Ni, Ruiyu <ruiyu...@intel.com>; Alcantara, Paulo <pa...@hp.com>; Carsey, 
Jaben <jaben.car...@intel.com>; edk2-devel@lists.01.org
Subject: RE: [PATCH] ShellPkg: Fix type mismatch with GCC

That's good. 

>-----Original Message-----
>From: Ni, Ruiyu
>Sent: Monday, October 30, 2017 1:25 PM
>To: Gao, Liming <liming....@intel.com>; Alcantara, Paulo 
><pa...@hp.com>; Carsey, Jaben <jaben.car...@intel.com>; 
>edk2-devel@lists.01.org
>Subject: RE: [PATCH] ShellPkg: Fix type mismatch with GCC
>
>I have a pending task to remove the dependency of InternalCharToUpper().
>Below two bugs require the same fix.
>664   [Shell] UnicodeCollation->StriColl() should be used to replace StrinCmp 
>in
>UefiShellLevel2CommandsLib
>294   Strnicmp() should use UNICODE_COLLATION.StrUpr() instead of
>converting char to upper case inself
>
>I think fixing them two is the right solution to go.
>
>
>Thanks/Ray
>
>> -----Original Message-----
>> From: Gao, Liming
>> Sent: Monday, October 30, 2017 1:18 PM
>> To: Alcantara, Paulo <pa...@hp.com>; Carsey, Jaben 
>> <jaben.car...@intel.com>; edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu <ruiyu...@intel.com>
>> Subject: RE: [PATCH] ShellPkg: Fix type mismatch with GCC
>>
>> I suggest to rename it and add its implementation in ShellPkg. We 
>> don't expect to use the internal function from another library or driver.
>>
>> Another way is to propose adding StrniCmp() API into BaseLib.
>>
>> Thanks
>> Liming
>> >-----Original Message-----
>> >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>> >Of Alcantara, Paulo
>> >Sent: Saturday, October 28, 2017 2:51 AM
>> >To: Carsey, Jaben <jaben.car...@intel.com>; edk2-devel@lists.01.org
>> >Cc: Ni, Ruiyu <ruiyu...@intel.com>
>> >Subject: Re: [edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
>> >
>> >Hi Jaben,
>> >
>> >No, we can't. InternalCharToUpper() is declared internally in 
>> >BaseLib and unexported.
>> >
>> >The comment above the declaration in UefiShellLevel2CommandsLib.c 
>> >explains it:
>> >
>> >/**
>> >  Be lazy and borrow from baselib.
>> >
>> >  @param[in] Char   The character to convert to upper case.
>> >
>> >  @return Char as an upper case character.
>> >**/
>> >
>> >Thanks,
>> >Paulo
>> >
>> >________________________________________
>> >From: Carsey, Jaben <jaben.car...@intel.com>
>> >Sent: Friday, October 27, 2017 4:33 PM
>> >To: Alcantara, Paulo; edk2-devel@lists.01.org
>> >Cc: Ni, Ruiyu
>> >Subject: RE: [PATCH] ShellPkg: Fix type mismatch with GCC
>> >
>> >Are we redefining a function from BaseLib?  Why not remove the 
>> >redundant definition instead of making it match?
>> >
>> >-Jaben
>> >
>> >> -----Original Message-----
>> >> From: Paulo Alcantara [mailto:pa...@hp.com]
>> >> Sent: Friday, October 27, 2017 9:24 AM
>> >> To: edk2-devel@lists.01.org
>> >> Cc: Paulo Alcantara <pa...@hp.com>; Carsey, Jaben 
>> >> <jaben.car...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>
>> >> Subject: [PATCH] ShellPkg: Fix type mismatch with GCC
>> >> Importance: High
>> >>
>> >> This patch fixes the following warning reported by GCC 6.3:
>> >>
>> >>
>> >/home/pcacjr/src/edk2.git/ShellPkg/Library/UefiShellLevel2CommandsLi
>> >b
>/
>> U
>> >> efiShellLevel2CommandsLib.c:271:1:
>> >> warning: type of 'InternalCharToUpper' does not match original 
>> >> decl aration [-Wlto-type-mismatch]  InternalCharToUpper (  ^
>> >> /home/pcacjr/src/edk2.git/MdePkg/Library/BaseLib/String.c:555:1: note:
>> >> 'InternalCharToUpper' was previously declared here 
>> >> InternalCharToUpper (  ^
>> >>
>> >> Cc: Jaben Carsey <jaben.car...@intel.com>
>> >> Cc: Ruiyu Ni <ruiyu...@intel.com>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Paulo Alcantara <pa...@hp.com>
>> >> ---
>> >>
>> >>
>> >ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
>Li
>> b
>> >> .c | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git
>> >>
>> >a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comman
>d
>> s
>> >> Lib.c
>> >>
>> >b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comman
>d
>> s
>> >> Lib.c
>> >> index 7948e53cfc..bab6631e15 100644
>> >> ---
>> >>
>> >a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comman
>d
>> s
>> >> Lib.c
>> >> +++
>> >>
>> >b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comman
>d
>> s
>> >> Lib.c
>> >> @@ -268,6 +268,7 @@ VerifyIntermediateDirectories (
>> >>    @return Char as an upper case character.
>> >>  **/
>> >>  CHAR16
>> >> +EFIAPI
>> >>  InternalCharToUpper (
>> >>    IN CONST CHAR16                    Char
>> >>    );
>> >> --
>> >> 2.11.0
>> >
>> >_______________________________________________
>> >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

Reply via email to