From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C59301A1EEF for ; Wed, 5 Oct 2016 14:17:28 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP; 05 Oct 2016 14:17:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,302,1473145200"; d="scan'208";a="1060973339" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga002.jf.intel.com with ESMTP; 05 Oct 2016 14:17:29 -0700 Received: from fmsmsx151.amr.corp.intel.com (10.18.125.4) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 5 Oct 2016 14:17:27 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.167]) by FMSMSX151.amr.corp.intel.com ([169.254.7.194]) with mapi id 14.03.0248.002; Wed, 5 Oct 2016 14:17:27 -0700 From: "Carsey, Jaben" To: Tim Lewis , "afish@apple.com" , Laszlo Ersek CC: edk2-devel-01 , Leif Lindholm , "Carsey, Jaben" Thread-Topic: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform Thread-Index: AdIekL1EjMpAqwOYRWKk8YKyvfLMLgAjbAlwAACkBtAAFfCNAAAA2Y4AAAEEU4AAAD2LAAAOXvpQ//+ShACAAAPuAIAAAEoAgAACDwCAAHKqMA== Date: Wed, 5 Oct 2016 21:17:27 +0000 Message-ID: References: <93F01BC9-4B02-467E-B900-65C6775BB0F3@apple.com> <618fd786-24d4-653c-d6b8-cb774654c644@redhat.com> <7236196A5DF6C040855A6D96F556A53F3F3F6D@msmail.insydesw.com.tw> <7236196A5DF6C040855A6D96F556A53F3F3FD7@msmail.insydesw.com.tw> In-Reply-To: <7236196A5DF6C040855A6D96F556A53F3F3FD7@msmail.insydesw.com.tw> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzI1OTJmNGUtNGYxMi00YThkLTk2NTUtNDAwYjBiZTRkOWQzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IktrVjJZcjN1YkJObDlJQzRabVBKNXVDQTZGWmhUKzRYb0tNSWttMk5LaUk9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.1.200.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:17:29 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable That is not enough. That gets called by the constructor. We need to not c= heck in either constructor or the constructor worker. That difference only matters for the shell binary itself. -Jaben > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Tim Lewis > Sent: Wednesday, October 05, 2016 2:07 PM > To: afish@apple.com; Laszlo Ersek > Cc: Carsey, Jaben ; edk2-devel-01 devel@ml01.01.org>; Leif Lindholm > Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source = on > the Reference Platform > Importance: High >=20 > If he has a DXE driver that consumes the shell lib (a valid case), then m= y > original suggestion stands: move it to ShellLibConstructorWorker. This is > where the ShellLib's internal pointers to the Shell protocol are initiali= zed. >=20 > Tim >=20 > -----Original Message----- > From: afish@apple.com [mailto:afish@apple.com] > Sent: Wednesday, October 05, 2016 1:59 PM > To: Laszlo Ersek > Cc: Tim Lewis ; Carsey, Jaben > ; Shah, Tapan ; Daniil > Egranov ; edk2-devel-01 devel@ml01.01.org>; Leif Lindholm > Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source = on > the Reference Platform >=20 >=20 > > On Oct 5, 2016, at 1:58 PM, Laszlo Ersek wrote: > > > > On 10/05/16 22:44, Tim Lewis wrote: > >> Jaben -- > >> > >> In which cases would you have the Shell protocol present and not have > the Unicode Collation protocol? > > > > That's exactly the question! > > > > According to the spec, at least if the system cannot boot from a disk > > device, then the collation protocol need not be present. > > > >> By my count, the Shell itself cannot function without it (see > ProcessCommandLine()). > > > > In which case though I don't understand why Daniil experiences this > > change (=3D commit 583448b441650) as a regression on Juno. If the shell > > couldn't function without the collation protocol even before commit > > 583448b441650, then the shell must never have run on Juno -- because > > it appears to lack collation -- and then this change cannot be a regres= sion. > > >=20 > Looks like he has a DXE Driver that consume the ShellLib. >=20 > Thanks, >=20 > Andrew Fish >=20 > > Thanks > > Laszlo > > > > > >> > >> Tim > >> > >> > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > >> Of Carsey, Jaben > >> Sent: Wednesday, October 05, 2016 1:35 PM > >> To: Shah, Tapan ; Laszlo Ersek > >> ; Andrew Fish ; Daniil Egranov > >> > >> Cc: Carsey, Jaben ; edk2-devel-01 > >> ; Leif Lindholm > >> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 > >> source on the Reference Platform > >> > >> That should work. We also need to initialize the variable to NULL in = the > constructor. > >> > >> Do we need to be case insensitive for this? Can we use memcmp instead > of the prototol? > >> > >> -Jaben > >> > >>> -----Original Message----- > >>> From: Shah, Tapan [mailto:tapandshah@hpe.com] > >>> Sent: Wednesday, October 05, 2016 1:25 PM > >>> To: Laszlo Ersek ; Andrew Fish > ; > >>> Daniil Egranov > >>> Cc: Carsey, Jaben ; edk2-devel-01 >>> devel@ml01.01.org>; Leif Lindholm > >>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 > >>> source on the Reference Platform > >>> Importance: High > >>> > >>> How about moving protocol locate from constructor to the actual > >>> function level and returning an error if it fails to locate (See atta= ched > patch file). > >>> > >>> Tapan > >>> > >>> -----Original Message----- > >>> From: Laszlo Ersek [mailto:lersek@redhat.com] > >>> Sent: Wednesday, October 05, 2016 3:18 PM > >>> To: Andrew Fish ; Daniil Egranov > >>> > >>> Cc: Carsey, Jaben ; edk2-devel-01 >>> devel@ml01.01.org>; Leif Lindholm ; Shah, > >>> Tapan > >>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 > >>> source on the Reference Platform > >>> > >>> On 10/05/16 21:48, Andrew Fish wrote: > >>>> > >>>>> On Oct 5, 2016, at 12:24 PM, Daniil Egranov > >>>>> > >>> wrote: > >>>>> > >>>>> I have the same ASSERT issue on Juno platform even the > >>>>> EnglishDxe.inf is > >>> included to the platform build. If UefiShellLib has such dependency > >>> on the protocol then according to EDKII Module Writer's Guide you > >>> need to specify the dependency on protocol in the module .inf to > >>> ensure the drivers proper load sequence. > >>>>> > >>>>> 8.6 Dependency Expressions > >>>>> A dependency expression specifies the protocols that the DXE > >>>>> driver requires to execute. In EDK II, it is specified in the > >>>>> [Depex] section of INF > >>> file. > >>>>> > >>>> > >>>> The Dependency Expression is for DXE Drivers that are dispatched by > >>>> the > >>> DXE Core. A UEFI Driver from an option ROM or an Application does > >>> not get dispatched by the dispatch and the Depex will not help. The > >>> Depex ends up being a section in the FV and it has nothing to do > >>> with the PE/COFF image of the an application or option ROM. > >>>> > >>>> IMHO the shell should try as hard as possible to function and > >>>> should not > >>> ASSERT if some newer Protocol is missing. > >>> > >>> (Background: commit 583448b441650.) > >>> > >>> In this specific case, the protocol dependency seems possible to > eliminate: > >>> > >>> - > Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c > >>> includes the CharToUpper() function, which is minimal -- could even > >>> be copied or moved over -- and can translate the ASCII subset of > >>> UCS-2, > >>> > >>> - once the ASCII characters of the input string have been translated > >>> to upper case, the result could be searched for / compared against > L"NULL" > >>> using BaseLib.h APIs. > >>> > >>> (Given that L"NULL" only contains characters from the ASCII subset, > >>> it suffices to upper-case ASCII chars in the input string. No > >>> non-ASCII character in the BMP can translate to ASCII N/U/L via > >>> upcasing, even with the real collation protocol (I trust), and no > >>> other ASCII characters than n/u/l will translate to N/U/L. This > >>> means that we won't miss an instance of NULL because we didn't > >>> upcate it (because it was non-ascii), and it also means we won't > >>> mistakenly match something non-NULL as NULL.) > >>> > >>> Just my two cents. > >>> > >>> Laszlo > >> > >> _______________________________________________ > >> 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 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel