public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] ShellPkg: Fix type mismatch with GCC
@ 2017-10-27 16:24 Paulo Alcantara
  2017-10-27 18:33 ` Carsey, Jaben
  0 siblings, 1 reply; 7+ messages in thread
From: Paulo Alcantara @ 2017-10-27 16:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: Paulo Alcantara, Jaben Carsey, Ruiyu Ni

This patch fixes the following warning reported by GCC 6.3:

/home/pcacjr/src/edk2.git/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.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.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <paulo@hp.com>
---
 ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
index 7948e53cfc..bab6631e15 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
@@ -268,6 +268,7 @@ VerifyIntermediateDirectories (
   @return Char as an upper case character.
 **/
 CHAR16
+EFIAPI
 InternalCharToUpper (
   IN CONST CHAR16                    Char
   );
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] ShellPkg: Fix type mismatch with GCC
  2017-10-27 16:24 [PATCH] ShellPkg: Fix type mismatch with GCC Paulo Alcantara
@ 2017-10-27 18:33 ` Carsey, Jaben
  2017-10-27 18:50   ` Alcantara, Paulo
  0 siblings, 1 reply; 7+ messages in thread
From: Carsey, Jaben @ 2017-10-27 18:33 UTC (permalink / raw)
  To: Paulo Alcantara, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu

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:paulo@hp.com]
> Sent: Friday, October 27, 2017 9:24 AM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <paulo@hp.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@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/UefiShellLevel2CommandsLib/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.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <paulo@hp.com>
> ---
> 
> ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib
> .c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> index 7948e53cfc..bab6631e15 100644
> ---
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> +++
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> @@ -268,6 +268,7 @@ VerifyIntermediateDirectories (
>    @return Char as an upper case character.
>  **/
>  CHAR16
> +EFIAPI
>  InternalCharToUpper (
>    IN CONST CHAR16                    Char
>    );
> --
> 2.11.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ShellPkg: Fix type mismatch with GCC
  2017-10-27 18:33 ` Carsey, Jaben
@ 2017-10-27 18:50   ` Alcantara, Paulo
  2017-10-30  5:18     ` Gao, Liming
  0 siblings, 1 reply; 7+ messages in thread
From: Alcantara, Paulo @ 2017-10-27 18:50 UTC (permalink / raw)
  To: Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu

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.carsey@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:paulo@hp.com]
> Sent: Friday, October 27, 2017 9:24 AM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <paulo@hp.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@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/UefiShellLevel2CommandsLib/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.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <paulo@hp.com>
> ---
>
> ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib
> .c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> index 7948e53cfc..bab6631e15 100644
> ---
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> +++
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> @@ -268,6 +268,7 @@ VerifyIntermediateDirectories (
>    @return Char as an upper case character.
>  **/
>  CHAR16
> +EFIAPI
>  InternalCharToUpper (
>    IN CONST CHAR16                    Char
>    );
> --
> 2.11.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ShellPkg: Fix type mismatch with GCC
  2017-10-27 18:50   ` Alcantara, Paulo
@ 2017-10-30  5:18     ` Gao, Liming
  2017-10-30  5:25       ` Ni, Ruiyu
  0 siblings, 1 reply; 7+ messages in thread
From: Gao, Liming @ 2017-10-30  5:18 UTC (permalink / raw)
  To: Alcantara, Paulo, Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu

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-bounces@lists.01.org] On Behalf Of
>Alcantara, Paulo
>Sent: Saturday, October 28, 2017 2:51 AM
>To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@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.carsey@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:paulo@hp.com]
>> Sent: Friday, October 27, 2017 9:24 AM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <paulo@hp.com>; Carsey, Jaben
>> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@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/UefiShellLevel2CommandsLib/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.carsey@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara <paulo@hp.com>
>> ---
>>
>>
>ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib
>> .c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git
>>
>a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
>> Lib.c
>>
>b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
>> Lib.c
>> index 7948e53cfc..bab6631e15 100644
>> ---
>>
>a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
>> Lib.c
>> +++
>>
>b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
>> 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ShellPkg: Fix type mismatch with GCC
  2017-10-30  5:18     ` Gao, Liming
@ 2017-10-30  5:25       ` Ni, Ruiyu
  2017-10-30  5:26         ` Gao, Liming
  0 siblings, 1 reply; 7+ messages in thread
From: Ni, Ruiyu @ 2017-10-30  5:25 UTC (permalink / raw)
  To: Gao, Liming, Alcantara, Paulo, Carsey, Jaben,
	edk2-devel@lists.01.org

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 <paulo@hp.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@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-bounces@lists.01.org] On Behalf Of
> >Alcantara, Paulo
> >Sent: Saturday, October 28, 2017 2:51 AM
> >To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
> >Cc: Ni, Ruiyu <ruiyu.ni@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.carsey@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:paulo@hp.com]
> >> Sent: Friday, October 27, 2017 9:24 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: Paulo Alcantara <paulo@hp.com>; Carsey, Jaben
> >> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@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/UefiShellLevel2CommandsLib/
> 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.carsey@intel.com>
> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Paulo Alcantara <paulo@hp.com>
> >> ---
> >>
> >>
> >ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLi
> b
> >> .c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git
> >>
> >a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command
> s
> >> Lib.c
> >>
> >b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command
> s
> >> Lib.c
> >> index 7948e53cfc..bab6631e15 100644
> >> ---
> >>
> >a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command
> s
> >> Lib.c
> >> +++
> >>
> >b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command
> 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ShellPkg: Fix type mismatch with GCC
  2017-10-30  5:25       ` Ni, Ruiyu
@ 2017-10-30  5:26         ` Gao, Liming
  2017-10-30 12:44           ` Alcantara, Paulo
  0 siblings, 1 reply; 7+ messages in thread
From: Gao, Liming @ 2017-10-30  5:26 UTC (permalink / raw)
  To: Ni, Ruiyu, Alcantara, Paulo, Carsey, Jaben,
	edk2-devel@lists.01.org

That's good. 

>-----Original Message-----
>From: Ni, Ruiyu
>Sent: Monday, October 30, 2017 1:25 PM
>To: Gao, Liming <liming.gao@intel.com>; Alcantara, Paulo <paulo@hp.com>;
>Carsey, Jaben <jaben.carsey@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 <paulo@hp.com>; Carsey, Jaben
>> <jaben.carsey@intel.com>; edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu <ruiyu.ni@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-bounces@lists.01.org] On Behalf Of
>> >Alcantara, Paulo
>> >Sent: Saturday, October 28, 2017 2:51 AM
>> >To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
>> >Cc: Ni, Ruiyu <ruiyu.ni@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.carsey@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:paulo@hp.com]
>> >> Sent: Friday, October 27, 2017 9:24 AM
>> >> To: edk2-devel@lists.01.org
>> >> Cc: Paulo Alcantara <paulo@hp.com>; Carsey, Jaben
>> >> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@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/UefiShellLevel2CommandsLib
>/
>> 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.carsey@intel.com>
>> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Paulo Alcantara <paulo@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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ShellPkg: Fix type mismatch with GCC
  2017-10-30  5:26         ` Gao, Liming
@ 2017-10-30 12:44           ` Alcantara, Paulo
  0 siblings, 0 replies; 7+ messages in thread
From: Alcantara, Paulo @ 2017-10-30 12:44 UTC (permalink / raw)
  To: Gao, Liming, Ni, Ruiyu, Carsey, Jaben, edk2-devel@lists.01.org

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.gao@intel.com] 
Sent: Monday, October 30, 2017 3:27 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; Alcantara, Paulo <paulo@hp.com>; Carsey, Jaben <jaben.carsey@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.gao@intel.com>; Alcantara, Paulo 
><paulo@hp.com>; Carsey, Jaben <jaben.carsey@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 <paulo@hp.com>; Carsey, Jaben 
>> <jaben.carsey@intel.com>; edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu <ruiyu.ni@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-bounces@lists.01.org] On Behalf 
>> >Of Alcantara, Paulo
>> >Sent: Saturday, October 28, 2017 2:51 AM
>> >To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
>> >Cc: Ni, Ruiyu <ruiyu.ni@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.carsey@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:paulo@hp.com]
>> >> Sent: Friday, October 27, 2017 9:24 AM
>> >> To: edk2-devel@lists.01.org
>> >> Cc: Paulo Alcantara <paulo@hp.com>; Carsey, Jaben 
>> >> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@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.carsey@intel.com>
>> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Paulo Alcantara <paulo@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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-10-30 12:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-27 16:24 [PATCH] ShellPkg: Fix type mismatch with GCC Paulo Alcantara
2017-10-27 18:33 ` Carsey, Jaben
2017-10-27 18:50   ` Alcantara, Paulo
2017-10-30  5:18     ` Gao, Liming
2017-10-30  5:25       ` Ni, Ruiyu
2017-10-30  5:26         ` Gao, Liming
2017-10-30 12:44           ` Alcantara, Paulo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox