public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Alcantara, Paulo" <paulo@hp.com>
To: "Gao, Liming" <liming.gao@intel.com>,
	"Ni, Ruiyu" <ruiyu.ni@intel.com>,
	"Carsey, Jaben" <jaben.carsey@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [PATCH] ShellPkg: Fix type mismatch with GCC
Date: Mon, 30 Oct 2017 12:44:30 +0000	[thread overview]
Message-ID: <AT5PR8401MB0706455504FEFF30CDEEC947CA590@AT5PR8401MB0706.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E17234D@SHSMSX104.ccr.corp.intel.com>

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


      reply	other threads:[~2017-10-30 12:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AT5PR8401MB0706455504FEFF30CDEEC947CA590@AT5PR8401MB0706.NAMPRD84.PROD.OUTLOOK.COM \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox