1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH] ShellPkg: Fix type mismatch with GCC

Paulo Alcantara posted 1 patch 7 years, 1 month ago
Failed in applying to current master (apply log)
ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c | 1 +
1 file changed, 1 insertion(+)
[edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
Posted by Paulo Alcantara 7 years, 1 month ago 
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
Posted by Carsey, Jaben 7 years, 1 month ago 
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
Re: [edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
Posted by Alcantara, Paulo 7 years, 1 month ago 
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
Re: [edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
Posted by Gao, Liming 7 years, 1 month ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
Posted by Ni, Ruiyu 7 years, 1 month ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
Posted by Gao, Liming 7 years, 1 month ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg: Fix type mismatch with GCC
Posted by Alcantara, Paulo 7 years, 1 month ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.