From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from msmail.insydesw.com.tw (ms.insydesw.com [211.75.113.220]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3C9F01A1E31 for ; Wed, 5 Oct 2016 14:06:51 -0700 (PDT) Received: from msmail.insydesw.com.tw ([fe80::74f7:f173:f4aa:9a05]) by msmail.insydesw.com.tw ([fe80::74f7:f173:f4aa:9a05%11]) with mapi id 14.01.0438.000; Thu, 6 Oct 2016 05:06:49 +0800 From: Tim Lewis To: "afish@apple.com" , Laszlo Ersek CC: "Carsey, Jaben" , "Shah, Tapan" , Daniil Egranov , edk2-devel-01 , Leif Lindholm Thread-Topic: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform Thread-Index: AdIekL1EjMpAqwOYRWKk8YKyvfLMLgAjbAlwAACkBtD//7QPAIAABswAgAAII4CAAAHsAIAAAr6A//94KYCAAI6EAIAAAEoA//94TQA= Date: Wed, 5 Oct 2016 21:06:48 +0000 Message-ID: <7236196A5DF6C040855A6D96F556A53F3F3FD7@msmail.insydesw.com.tw> References: <93F01BC9-4B02-467E-B900-65C6775BB0F3@apple.com> <618fd786-24d4-653c-d6b8-cb774654c644@redhat.com> <7236196A5DF6C040855A6D96F556A53F3F3F6D@msmail.insydesw.com.tw> In-Reply-To: Accept-Language: en-US, zh-TW X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [192.168.100.107] MIME-Version: 1.0 Subject: Re: Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Oct 2016 21:06:51 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable If he has a DXE driver that consumes the shell lib (a valid case), then my = original suggestion stands: move it to ShellLibConstructorWorker. This is w= here the ShellLib's internal pointers to the Shell protocol are initialized= . Tim -----Original Message----- From: afish@apple.com [mailto:afish@apple.com]=20 Sent: Wednesday, October 05, 2016 1:59 PM To: Laszlo Ersek Cc: Tim Lewis ; Carsey, Jaben ; Shah, Tapan ; Daniil Egranov ; edk2-devel-01 ; Leif Lindholm Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on= the Reference Platform > On Oct 5, 2016, at 1:58 PM, Laszlo Ersek wrote: >=20 > On 10/05/16 22:44, Tim Lewis wrote: >> Jaben -- >>=20 >> In which cases would you have the Shell protocol present and not have th= e Unicode Collation protocol? >=20 > That's exactly the question! >=20 > According to the spec, at least if the system cannot boot from a disk=20 > device, then the collation protocol need not be present. >=20 >> By my count, the Shell itself cannot function without it (see ProcessCom= mandLine()).=20 >=20 > In which case though I don't understand why Daniil experiences this=20 > change (=3D commit 583448b441650) as a regression on Juno. If the shell=20 > couldn't function without the collation protocol even before commit=20 > 583448b441650, then the shell must never have run on Juno -- because=20 > it appears to lack collation -- and then this change cannot be a regressi= on. >=20 Looks like he has a DXE Driver that consume the ShellLib.=20 Thanks, Andrew Fish > Thanks > Laszlo >=20 >=20 >>=20 >> Tim >>=20 >>=20 >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf=20 >> Of Carsey, Jaben >> Sent: Wednesday, October 05, 2016 1:35 PM >> To: Shah, Tapan ; Laszlo Ersek=20 >> ; Andrew Fish ; Daniil Egranov=20 >> >> Cc: Carsey, Jaben ; edk2-devel-01=20 >> ; Leif Lindholm >> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2=20 >> source on the Reference Platform >>=20 >> That should work. We also need to initialize the variable to NULL in th= e constructor. >>=20 >> Do we need to be case insensitive for this? Can we use memcmp instead o= f the prototol? >>=20 >> -Jaben >>=20 >>> -----Original Message----- >>> From: Shah, Tapan [mailto:tapandshah@hpe.com] >>> Sent: Wednesday, October 05, 2016 1:25 PM >>> To: Laszlo Ersek ; Andrew Fish ;=20 >>> Daniil Egranov >>> Cc: Carsey, Jaben ; edk2-devel-01 >> devel@ml01.01.org>; Leif Lindholm >>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2=20 >>> source on the Reference Platform >>> Importance: High >>>=20 >>> How about moving protocol locate from constructor to the actual=20 >>> function level and returning an error if it fails to locate (See attach= ed patch file). >>>=20 >>> Tapan >>>=20 >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Wednesday, October 05, 2016 3:18 PM >>> To: Andrew Fish ; Daniil Egranov=20 >>> >>> Cc: Carsey, Jaben ; edk2-devel-01 >> devel@ml01.01.org>; Leif Lindholm ; Shah,=20 >>> Tapan >>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2=20 >>> source on the Reference Platform >>>=20 >>> On 10/05/16 21:48, Andrew Fish wrote: >>>>=20 >>>>> On Oct 5, 2016, at 12:24 PM, Daniil Egranov=20 >>>>> >>> wrote: >>>>>=20 >>>>> I have the same ASSERT issue on Juno platform even the=20 >>>>> EnglishDxe.inf is >>> included to the platform build. If UefiShellLib has such dependency=20 >>> on the protocol then according to EDKII Module Writer's Guide you=20 >>> need to specify the dependency on protocol in the module .inf to=20 >>> ensure the drivers proper load sequence. >>>>>=20 >>>>> 8.6 Dependency Expressions >>>>> A dependency expression specifies the protocols that the DXE=20 >>>>> driver requires to execute. In EDK II, it is specified in the=20 >>>>> [Depex] section of INF >>> file. >>>>>=20 >>>>=20 >>>> The Dependency Expression is for DXE Drivers that are dispatched by=20 >>>> the >>> DXE Core. A UEFI Driver from an option ROM or an Application does=20 >>> not get dispatched by the dispatch and the Depex will not help. The=20 >>> Depex ends up being a section in the FV and it has nothing to do=20 >>> with the PE/COFF image of the an application or option ROM. >>>>=20 >>>> IMHO the shell should try as hard as possible to function and=20 >>>> should not >>> ASSERT if some newer Protocol is missing. >>>=20 >>> (Background: commit 583448b441650.) >>>=20 >>> In this specific case, the protocol dependency seems possible to elimin= ate: >>>=20 >>> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c >>> includes the CharToUpper() function, which is minimal -- could even=20 >>> be copied or moved over -- and can translate the ASCII subset of=20 >>> UCS-2, >>>=20 >>> - once the ASCII characters of the input string have been translated=20 >>> to upper case, the result could be searched for / compared against L"NU= LL" >>> using BaseLib.h APIs. >>>=20 >>> (Given that L"NULL" only contains characters from the ASCII subset,=20 >>> it suffices to upper-case ASCII chars in the input string. No=20 >>> non-ASCII character in the BMP can translate to ASCII N/U/L via=20 >>> upcasing, even with the real collation protocol (I trust), and no=20 >>> other ASCII characters than n/u/l will translate to N/U/L. This=20 >>> means that we won't miss an instance of NULL because we didn't=20 >>> upcate it (because it was non-ascii), and it also means we won't=20 >>> mistakenly match something non-NULL as NULL.) >>>=20 >>> Just my two cents. >>>=20 >>> Laszlo >>=20 >> _______________________________________________ >> 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 >>=20 >=20