From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.20; helo=mga02.intel.com; envelope-from=jaben.carsey@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A9F11222DE15F for ; Mon, 12 Feb 2018 08:56:56 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Feb 2018 09:02:45 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,503,1511856000"; d="scan'208";a="34088140" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga002.jf.intel.com with ESMTP; 12 Feb 2018 09:02:44 -0800 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 12 Feb 2018 09:02:44 -0800 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.47]) by fmsmsx120.amr.corp.intel.com ([169.254.15.251]) with mapi id 14.03.0319.002; Mon, 12 Feb 2018 09:02:44 -0800 From: "Carsey, Jaben" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH] ShellPkg/help: Fix "-?" may not show manual sometimes Shell core was enhanced to find the manual string in PE resource section. But the finding algorithm is too strict: If the manual is written beginning with: .TH command 0 "descripton of comm Thread-Index: AdOkI0orCJCZEwzaR9GsHkwom7YH0A== Date: Mon, 12 Feb 2018 17:02:43 +0000 Message-ID: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiN2ViZjM5ZDAtNWQ0Yi00OGI1LWIwZWItYTk2YTRlZDkxNzE3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IlZvc3p2MEZZN3hsb0JYSW00NkZHVFhtQmFLOWI4UDBVVFRUOGkzUGZwTDQ9In0= x-ctpclassification: CTP_NT x-originating-ip: [10.1.200.107] MIME-Version: 1.0 Subject: Re: [PATCH] ShellPkg/help: Fix "-?" may not show manual sometimes Shell core was enhanced to find the manual string in PE resource section. But the finding algorithm is too strict: If the manual is written beginning with: .TH command 0 "descripton of comm X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Feb 2018 16:56:57 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Jaben Carsey > -----Original Message----- > From: Ni, Ruiyu > Sent: Sunday, February 11, 2018 7:18 AM > To: edk2-devel@lists.01.org > Cc: Carsey, Jaben > Subject: [PATCH] ShellPkg/help: Fix "-?" may not show manual sometimes > Shell core was enhanced to find the manual string in PE resource section.= But > the finding algorithm is too strict: If the manual is written beginning w= ith: .TH > command 0 "descripton of command" > Importance: High >=20 > but user types "COMMAND.efi -?". The finding algorithm uses > case-sensitive compare between "command" and "COMMAND" resulting > in the manual cannot be found. >=20 > The patch fixes this issue by using existing ManFileFindTitleSection > and ManFileFindSections which compare command case-insensitive. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ruiyu Ni > Cc: Jaben Carsey > --- > ShellPkg/Application/Shell/FileHandleWrappers.c | 52 ++++- > ShellPkg/Application/Shell/ShellManParser.c | 275 ++----------------= ------ > 2 files changed, 73 insertions(+), 254 deletions(-) >=20 > diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c > b/ShellPkg/Application/Shell/FileHandleWrappers.c > index 0a7a60294d..63aad69fe8 100644 > --- a/ShellPkg/Application/Shell/FileHandleWrappers.c > +++ b/ShellPkg/Application/Shell/FileHandleWrappers.c > @@ -3,7 +3,7 @@ > StdIn, StdOut, StdErr, etc...). >=20 > Copyright 2016 Dell Inc. > - Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> (C) Copyright 2013 Hewlett-Packard Development Company, L.P.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > @@ -1554,6 +1554,54 @@ FileInterfaceMemGetPosition( > return (EFI_SUCCESS); > } >=20 > +/** > + File style interface for Mem (GetInfo). > + > + @param This Protocol instance pointer. > + @param InformationType Type of information to return in Buffer. > + @param BufferSize On input size of buffer, on output amount of d= ata in > buffer. > + @param Buffer The buffer to return data. > + > + @retval EFI_SUCCESS Data was returned. > + @retval EFI_UNSUPPORT InformationType is not supported. > + @retval EFI_NO_MEDIA The device has no media. > + @retval EFI_DEVICE_ERROR The device reported an error. > + @retval EFI_VOLUME_CORRUPTED The file system structures are > corrupted. > + @retval EFI_WRITE_PROTECTED The device is write protected. > + @retval EFI_ACCESS_DENIED The file was open for read only. > + @retval EFI_BUFFER_TOO_SMALL Buffer was too small; required size > returned in BufferSize. > + > +**/ > +EFI_STATUS > +EFIAPI > +FileInterfaceMemGetInfo( > + IN EFI_FILE_PROTOCOL *This, > + IN EFI_GUID *InformationType, > + IN OUT UINTN *BufferSize, > + OUT VOID *Buffer > + ) > +{ > + EFI_FILE_INFO *FileInfo; > + > + if (CompareGuid (InformationType, &gEfiFileInfoGuid)) { > + if (*BufferSize < sizeof (EFI_FILE_INFO)) { > + *BufferSize =3D sizeof (EFI_FILE_INFO); > + return EFI_BUFFER_TOO_SMALL; > + } > + if (Buffer =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + FileInfo =3D (EFI_FILE_INFO *)Buffer; > + FileInfo->Size =3D sizeof (*FileInfo); > + ZeroMem (FileInfo, sizeof (*FileInfo)); > + FileInfo->FileSize =3D ((EFI_FILE_PROTOCOL_MEM*)This)->FileSize; > + FileInfo->PhysicalSize =3D FileInfo->FileSize; > + return EFI_SUCCESS; > + } > + > + return EFI_UNSUPPORTED; > +} > + > /** > File style interface for Mem (Write). >=20 > @@ -1689,7 +1737,7 @@ CreateFileInterfaceMem( > FileInterface->Close =3D FileInterfaceMemClose; > FileInterface->GetPosition =3D FileInterfaceMemGetPosition; > FileInterface->SetPosition =3D FileInterfaceMemSetPosition; > - FileInterface->GetInfo =3D FileInterfaceNopGetInfo; > + FileInterface->GetInfo =3D FileInterfaceMemGetInfo; > FileInterface->SetInfo =3D FileInterfaceNopSetInfo; > FileInterface->Flush =3D FileInterfaceNopGeneric; > FileInterface->Delete =3D FileInterfaceNopGeneric; > diff --git a/ShellPkg/Application/Shell/ShellManParser.c > b/ShellPkg/Application/Shell/ShellManParser.c > index 7a290e16f6..975f3c22da 100644 > --- a/ShellPkg/Application/Shell/ShellManParser.c > +++ b/ShellPkg/Application/Shell/ShellManParser.c > @@ -1,7 +1,7 @@ > /** @file > Provides interface to shell MAN file parser. >=20 > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> Copyright 2015 Dell Inc. > This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > @@ -205,138 +205,6 @@ SearchPathForFile( > return (Status); > } >=20 > -/** > - parses through Buffer (which is MAN file formatted) and returns the > - detailed help for any sub section specified in the comma seperated lis= t of > - sections provided. If the end of the file or a .TH section is found t= hen > - return. > - > - Upon a sucessful return the caller is responsible to free the memory i= n > *HelpText > - > - @param[in] Buffer Buffer to read from > - @param[in] Sections name of command's sub sections to find > - @param[in] HelpText pointer to pointer to string where text = goes. > - @param[in] HelpSize pointer to size of allocated HelpText (m= ay be > updated) > - > - @retval EFI_OUT_OF_RESOURCES a memory allocation failed. > - @retval EFI_SUCCESS the section was found and its descriptio= n sotred > in > - an alloceted buffer. > -**/ > -EFI_STATUS > -ManBufferFindSections( > - IN CONST CHAR16 *Buffer, > - IN CONST CHAR16 *Sections, > - IN CHAR16 **HelpText, > - IN UINTN *HelpSize > - ) > -{ > - EFI_STATUS Status; > - CONST CHAR16 *CurrentLocation; > - BOOLEAN CurrentlyReading; > - CHAR16 *SectionName; > - UINTN SectionLen; > - BOOLEAN Found; > - CHAR16 *TempString; > - CHAR16 *TempString2; > - > - if ( Buffer =3D=3D NULL > - || HelpText =3D=3D NULL > - || HelpSize =3D=3D NULL > - ){ > - return (EFI_INVALID_PARAMETER); > - } > - > - Status =3D EFI_SUCCESS; > - CurrentlyReading =3D FALSE; > - Found =3D FALSE; > - > - for (CurrentLocation =3D Buffer,TempString =3D NULL > - ; CurrentLocation !=3D NULL && *CurrentLocation !=3D CHAR_NULL > - ; CurrentLocation=3DStrStr(CurrentLocation, L"\r\n"),TempString =3D= NULL > - ){ > - while(CurrentLocation[0] =3D=3D L'\r' || CurrentLocation[0] =3D=3D L= '\n') { > - CurrentLocation++; > - } > - if (CurrentLocation[0] =3D=3D L'#') { > - // > - // Skip comment lines > - // > - continue; > - } > - if (StrnCmp(CurrentLocation, L".TH", 3) =3D=3D 0) { > - // > - // we hit the end of this commands section so stop. > - // > - break; > - } > - if (StrnCmp(CurrentLocation, L".SH ", 4) =3D=3D 0) { > - if (Sections =3D=3D NULL) { > - CurrentlyReading =3D TRUE; > - continue; > - } else if (CurrentlyReading) { > - CurrentlyReading =3D FALSE; > - } > - CurrentLocation +=3D 4; > - // > - // is this a section we want to read in? > - // > - if (StrLen(CurrentLocation)!=3D0) { > - TempString2 =3D StrStr(CurrentLocation, L" "); > - TempString2 =3D MIN(TempString2, StrStr(CurrentLocation, L"\r"))= ; > - TempString2 =3D MIN(TempString2, StrStr(CurrentLocation, L"\n"))= ; > - ASSERT(TempString =3D=3D NULL); > - TempString =3D StrnCatGrow(&TempString, NULL, CurrentLocation, > TempString2=3D=3DNULL?0:TempString2 - CurrentLocation); > - if (TempString =3D=3D NULL) { > - Status =3D EFI_OUT_OF_RESOURCES; > - break; > - } > - SectionName =3D TempString; > - SectionLen =3D StrLen(SectionName); > - SectionName =3D StrStr(Sections, SectionName); > - if (SectionName =3D=3D NULL) { > - SHELL_FREE_NON_NULL(TempString); > - continue; > - } > - if (*(SectionName + SectionLen) =3D=3D CHAR_NULL || *(SectionNam= e + > SectionLen) =3D=3D L',') { > - CurrentlyReading =3D TRUE; > - } > - } > - } else if (CurrentlyReading) { > - Found =3D TRUE; > - if (StrLen(CurrentLocation)!=3D0) { > - TempString2 =3D StrStr(CurrentLocation, L"\r"); > - TempString2 =3D MIN(TempString2, StrStr(CurrentLocation, L"\n"))= ; > - ASSERT(TempString =3D=3D NULL); > - TempString =3D StrnCatGrow(&TempString, NULL, CurrentLocation, > TempString2=3D=3DNULL?0:TempString2 - CurrentLocation); > - if (TempString =3D=3D NULL) { > - Status =3D EFI_OUT_OF_RESOURCES; > - break; > - } > - // > - // copy and save the current line. > - // > - ASSERT((*HelpText =3D=3D NULL && *HelpSize =3D=3D 0) || (*HelpTe= xt !=3D > NULL)); > - StrnCatGrow (HelpText, HelpSize, TempString, 0); > - if (HelpText =3D=3D NULL) { > - Status =3D EFI_OUT_OF_RESOURCES; > - break; > - } > - StrnCatGrow (HelpText, HelpSize, L"\r\n", 0); > - if (HelpText =3D=3D NULL) { > - Status =3D EFI_OUT_OF_RESOURCES; > - break; > - } > - } > - } > - SHELL_FREE_NON_NULL(TempString); > - } > - SHELL_FREE_NON_NULL(TempString); > - if (!Found && !EFI_ERROR(Status)) { > - return (EFI_NOT_FOUND); > - } > - return (Status); > -} > - > /** > parses through the MAN file specified by SHELL_FILE_HANDLE and returns > the > detailed help for any sub section specified in the comma seperated lis= t of > @@ -452,111 +320,6 @@ ManFileFindSections( > return (Status); > } >=20 > -/** > - parses through the MAN file formatted Buffer and returns the > - "Brief Description" for the .TH section as specified by Command. If t= he > - command section is not found return EFI_NOT_FOUND. > - > - Upon a sucessful return the caller is responsible to free the memory i= n > *BriefDesc > - > - @param[in] Buffer Buffer to read from > - @param[in] Command name of command's section to find > - @param[in] BriefDesc pointer to pointer to string where descr= iption > goes. > - @param[in] BriefSize pointer to size of allocated BriefDesc > - > - @retval EFI_OUT_OF_RESOURCES a memory allocation failed. > - @retval EFI_SUCCESS the section was found and its descriptio= n sotred > in > - an alloceted buffer. > -**/ > -EFI_STATUS > -ManBufferFindTitleSection( > - IN CHAR16 **Buffer, > - IN CONST CHAR16 *Command, > - IN CHAR16 **BriefDesc, > - IN UINTN *BriefSize > - ) > -{ > - EFI_STATUS Status; > - CHAR16 *TitleString; > - CHAR16 *TitleEnd; > - CHAR16 *CurrentLocation; > - UINTN TitleLength; > - UINTN Start; > - CONST CHAR16 StartString[] =3D L".TH "; > - CONST CHAR16 EndString[] =3D L" 0 "; > - > - if ( Buffer =3D=3D NULL > - || Command =3D=3D NULL > - || (BriefDesc !=3D NULL && BriefSize =3D=3D NULL) > - ){ > - return (EFI_INVALID_PARAMETER); > - } > - > - Status =3D EFI_SUCCESS; > - > - // > - // Do not pass any leading path information that may be present to > IsTitleHeader(). > - // > - Start =3D StrLen(Command); > - while ((Start !=3D 0) > - && (*(Command + Start - 1) !=3D L'\\') > - && (*(Command + Start - 1) !=3D L'/') > - && (*(Command + Start - 1) !=3D L':')) { > - --Start; > - } > - > - // > - // more characters for StartString and EndString > - // > - TitleLength =3D StrSize(Command + Start) + (StrLen(StartString) + > StrLen(EndString)) * sizeof(CHAR16); > - TitleString =3D AllocateZeroPool(TitleLength); > - if (TitleString =3D=3D NULL) { > - return (EFI_OUT_OF_RESOURCES); > - } > - StrCpyS(TitleString, TitleLength/sizeof(CHAR16), StartString); > - StrCatS(TitleString, TitleLength/sizeof(CHAR16), Command + Start); > - StrCatS(TitleString, TitleLength/sizeof(CHAR16), EndString); > - > - CurrentLocation =3D StrStr(*Buffer, TitleString); > - if (CurrentLocation =3D=3D NULL){ > - Status =3D EFI_NOT_FOUND; > - } else { > - // > - // we found it so copy out the rest of the line into BriefDesc > - // After skipping any spaces or zeroes > - // > - for (CurrentLocation +=3D StrLen(TitleString) > - ; *CurrentLocation =3D=3D L' ' || *CurrentLocation =3D=3D L'0' ||= *CurrentLocation > =3D=3D L'1' || *CurrentLocation =3D=3D L'\"' > - ; CurrentLocation++); > - > - TitleEnd =3D StrStr(CurrentLocation, L"\""); > - if (TitleEnd =3D=3D NULL) { > - Status =3D EFI_DEVICE_ERROR; > - } else { > - if (BriefDesc !=3D NULL) { > - *BriefSize =3D StrSize(TitleEnd); > - *BriefDesc =3D AllocateZeroPool(*BriefSize); > - if (*BriefDesc =3D=3D NULL) { > - Status =3D EFI_OUT_OF_RESOURCES; > - } else { > - StrnCpyS(*BriefDesc, (*BriefSize)/sizeof(CHAR16), CurrentLocat= ion, > TitleEnd-CurrentLocation); > - } > - } > - > - for (CurrentLocation =3D TitleEnd > - ; *CurrentLocation !=3D L'\n' > - ; CurrentLocation++); > - for ( > - ; *CurrentLocation =3D=3D L' ' || *CurrentLocation =3D=3D L'\n'= || > *CurrentLocation =3D=3D L'\r' > - ; CurrentLocation++); > - *Buffer =3D CurrentLocation; > - } > - } > - > - FreePool(TitleString); > - return (Status); > -} > - > /** > Parses a line from a MAN file to see if it is the Title Header. If it = is, then > if the "Brief Description" is desired, allocate a buffer for it and re= turn a > @@ -813,10 +576,8 @@ ProcessManFile( > UINTN BriefSize; > UINTN StringIdWalker; > BOOLEAN Ascii; > - CHAR16 *TempString2; > CHAR16 *CmdFileName; > CHAR16 *CmdFilePathName; > - CHAR16 *StringBuff; > EFI_DEVICE_PATH_PROTOCOL *FileDevPath; > EFI_DEVICE_PATH_PROTOCOL *DevPath; > EFI_HII_PACKAGE_LIST_HEADER *PackageListHeader; > @@ -836,7 +597,6 @@ ProcessManFile( > CmdFileName =3D NULL; > CmdFilePathName =3D NULL; > CmdFileImgHandle =3D NULL; > - StringBuff =3D NULL; > PackageListHeader =3D NULL; > FileDevPath =3D NULL; > DevPath =3D NULL; > @@ -846,11 +606,17 @@ ProcessManFile( > // > TempString =3D ShellCommandGetCommandHelp(Command); > if (TempString !=3D NULL) { > - TempString2 =3D TempString; > - Status =3D ManBufferFindTitleSection(&TempString2, Command, BriefDes= c, > &BriefSize); > + FileHandle =3D ConvertEfiFileProtocolToShellHandle > (CreateFileInterfaceMem (TRUE), NULL); > + HelpSize =3D StrLen (TempString) * sizeof (CHAR16); > + ShellWriteFile (FileHandle, &HelpSize, TempString); > + ShellSetFilePosition (FileHandle, 0); > + HelpSize =3D 0; > + BriefSize =3D 0; > + Status =3D ManFileFindTitleSection(FileHandle, Command, BriefDesc, > &BriefSize, &Ascii); > if (!EFI_ERROR(Status) && HelpText !=3D NULL){ > - Status =3D ManBufferFindSections(TempString2, Sections, HelpText, > &HelpSize); > + Status =3D ManFileFindSections(FileHandle, Sections, HelpText, &He= lpSize, > Ascii); > } > + ShellCloseFile (&FileHandle); > } else { > // > // If the image is a external app, check .MAN file first. > @@ -947,20 +713,26 @@ ProcessManFile( >=20 > StringIdWalker =3D 1; > do { > - SHELL_FREE_NON_NULL(StringBuff); > + SHELL_FREE_NON_NULL(TempString); > if (BriefDesc !=3D NULL) { > SHELL_FREE_NON_NULL(*BriefDesc); > } > - StringBuff =3D HiiGetString (mShellManHiiHandle, > (EFI_STRING_ID)StringIdWalker, NULL); > - if (StringBuff =3D=3D NULL) { > + TempString =3D HiiGetString (mShellManHiiHandle, > (EFI_STRING_ID)StringIdWalker, NULL); > + if (TempString =3D=3D NULL) { > Status =3D EFI_NOT_FOUND; > goto Done; > } > - TempString2 =3D StringBuff; > - Status =3D ManBufferFindTitleSection(&TempString2, Command, > BriefDesc, &BriefSize); > + FileHandle =3D ConvertEfiFileProtocolToShellHandle > (CreateFileInterfaceMem (TRUE), NULL); > + HelpSize =3D StrLen (TempString) * sizeof (CHAR16); > + ShellWriteFile (FileHandle, &HelpSize, TempString); > + ShellSetFilePosition (FileHandle, 0); > + HelpSize =3D 0; > + BriefSize =3D 0; > + Status =3D ManFileFindTitleSection(FileHandle, Command, BriefDes= c, > &BriefSize, &Ascii); > if (!EFI_ERROR(Status) && HelpText !=3D NULL){ > - Status =3D ManBufferFindSections(TempString2, Sections, HelpTe= xt, > &HelpSize); > + Status =3D ManFileFindSections(FileHandle, Sections, HelpText, > &HelpSize, Ascii); > } > + ShellCloseFile (&FileHandle); > if (!EFI_ERROR(Status)){ > // > // Found what we need and return > @@ -969,7 +741,7 @@ ProcessManFile( > } >=20 > StringIdWalker +=3D 1; > - } while (StringIdWalker < 0xFFFF && StringBuff !=3D NULL); > + } while (StringIdWalker < 0xFFFF && TempString !=3D NULL); >=20 > } >=20 > @@ -992,7 +764,6 @@ Done: > Status =3D gBS->UnloadImage (CmdFileImgHandle); > } >=20 > - SHELL_FREE_NON_NULL(StringBuff); > SHELL_FREE_NON_NULL(TempString); > SHELL_FREE_NON_NULL(CmdFileName); > SHELL_FREE_NON_NULL(CmdFilePathName); > -- > 2.16.1.windows.1