I think that is fine. I don’t think that there is a difference except the ShellCloseFile() is only available inside the shell, while the protocol API is available to applications that open the ShellProtocol.
-Jaben > -----Original Message----- > From: Ni, Ruiyu > Sent: Sunday, February 11, 2018 7:20 AM > To: Carsey, Jaben <[email protected]>; [email protected] > Subject: Re: [edk2] [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 "descript... > Importance: High > > Jaben, > I am not sure whether calling ShellCloseFile() in below code is proper. > I tested the change and didn't find any issue. > > But I found the existing code uses: > ShellInfoObject.NewEfiShellProtocol->CloseFile(FileHandle); > > I am not sure what's the difference. > > On 2/11/2018 11:17 PM, Ruiyu Ni wrote: > > but user types "COMMAND.efi -?". The finding algorithm uses > > case-sensitive compare between "command" and "COMMAND" resulting > > in the manual cannot be found. > > > > The patch fixes this issue by using existing ManFileFindTitleSection > > and ManFileFindSections which compare command case-insensitive. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ruiyu Ni <[email protected]> > > Cc: Jaben Carsey <[email protected]> > > --- > > ShellPkg/Application/Shell/FileHandleWrappers.c | 52 ++++- > > ShellPkg/Application/Shell/ShellManParser.c | 275 > > ++---------------------- > > 2 files changed, 73 insertions(+), 254 deletions(-) > > > > 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...). > > > > Copyright 2016 Dell Inc. > > - Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR> > > + Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR> > > (C) Copyright 2013 Hewlett-Packard Development Company, L.P.<BR> > > 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); > > } > > > > +/** > > + 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 data > 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 = sizeof (EFI_FILE_INFO); > > + return EFI_BUFFER_TOO_SMALL; > > + } > > + if (Buffer == NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + FileInfo = (EFI_FILE_INFO *)Buffer; > > + FileInfo->Size = sizeof (*FileInfo); > > + ZeroMem (FileInfo, sizeof (*FileInfo)); > > + FileInfo->FileSize = ((EFI_FILE_PROTOCOL_MEM*)This)->FileSize; > > + FileInfo->PhysicalSize = FileInfo->FileSize; > > + return EFI_SUCCESS; > > + } > > + > > + return EFI_UNSUPPORTED; > > +} > > + > > /** > > File style interface for Mem (Write). > > > > @@ -1689,7 +1737,7 @@ CreateFileInterfaceMem( > > FileInterface->Close = FileInterfaceMemClose; > > FileInterface->GetPosition = FileInterfaceMemGetPosition; > > FileInterface->SetPosition = FileInterfaceMemSetPosition; > > - FileInterface->GetInfo = FileInterfaceNopGetInfo; > > + FileInterface->GetInfo = FileInterfaceMemGetInfo; > > FileInterface->SetInfo = FileInterfaceNopSetInfo; > > FileInterface->Flush = FileInterfaceNopGeneric; > > FileInterface->Delete = 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. > > > > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR> > > + Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR> > > 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); > > } > > > > -/** > > - parses through Buffer (which is MAN file formatted) and returns the > > - detailed help for any sub section specified in the comma seperated list > > of > > - sections provided. If the end of the file or a .TH section is found then > > - return. > > - > > - Upon a sucessful return the caller is responsible to free the memory in > *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 (may > > be > updated) > > - > > - @retval EFI_OUT_OF_RESOURCES a memory allocation failed. > > - @retval EFI_SUCCESS the section was found and its description > 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 == NULL > > - || HelpText == NULL > > - || HelpSize == NULL > > - ){ > > - return (EFI_INVALID_PARAMETER); > > - } > > - > > - Status = EFI_SUCCESS; > > - CurrentlyReading = FALSE; > > - Found = FALSE; > > - > > - for (CurrentLocation = Buffer,TempString = NULL > > - ; CurrentLocation != NULL && *CurrentLocation != CHAR_NULL > > - ; CurrentLocation=StrStr(CurrentLocation, L"\r\n"),TempString = NULL > > - ){ > > - while(CurrentLocation[0] == L'\r' || CurrentLocation[0] == L'\n') { > > - CurrentLocation++; > > - } > > - if (CurrentLocation[0] == L'#') { > > - // > > - // Skip comment lines > > - // > > - continue; > > - } > > - if (StrnCmp(CurrentLocation, L".TH", 3) == 0) { > > - // > > - // we hit the end of this commands section so stop. > > - // > > - break; > > - } > > - if (StrnCmp(CurrentLocation, L".SH ", 4) == 0) { > > - if (Sections == NULL) { > > - CurrentlyReading = TRUE; > > - continue; > > - } else if (CurrentlyReading) { > > - CurrentlyReading = FALSE; > > - } > > - CurrentLocation += 4; > > - // > > - // is this a section we want to read in? > > - // > > - if (StrLen(CurrentLocation)!=0) { > > - TempString2 = StrStr(CurrentLocation, L" "); > > - TempString2 = MIN(TempString2, StrStr(CurrentLocation, L"\r")); > > - TempString2 = MIN(TempString2, StrStr(CurrentLocation, L"\n")); > > - ASSERT(TempString == NULL); > > - TempString = StrnCatGrow(&TempString, NULL, CurrentLocation, > TempString2==NULL?0:TempString2 - CurrentLocation); > > - if (TempString == NULL) { > > - Status = EFI_OUT_OF_RESOURCES; > > - break; > > - } > > - SectionName = TempString; > > - SectionLen = StrLen(SectionName); > > - SectionName = StrStr(Sections, SectionName); > > - if (SectionName == NULL) { > > - SHELL_FREE_NON_NULL(TempString); > > - continue; > > - } > > - if (*(SectionName + SectionLen) == CHAR_NULL || *(SectionName + > SectionLen) == L',') { > > - CurrentlyReading = TRUE; > > - } > > - } > > - } else if (CurrentlyReading) { > > - Found = TRUE; > > - if (StrLen(CurrentLocation)!=0) { > > - TempString2 = StrStr(CurrentLocation, L"\r"); > > - TempString2 = MIN(TempString2, StrStr(CurrentLocation, L"\n")); > > - ASSERT(TempString == NULL); > > - TempString = StrnCatGrow(&TempString, NULL, CurrentLocation, > TempString2==NULL?0:TempString2 - CurrentLocation); > > - if (TempString == NULL) { > > - Status = EFI_OUT_OF_RESOURCES; > > - break; > > - } > > - // > > - // copy and save the current line. > > - // > > - ASSERT((*HelpText == NULL && *HelpSize == 0) || (*HelpText != > NULL)); > > - StrnCatGrow (HelpText, HelpSize, TempString, 0); > > - if (HelpText == NULL) { > > - Status = EFI_OUT_OF_RESOURCES; > > - break; > > - } > > - StrnCatGrow (HelpText, HelpSize, L"\r\n", 0); > > - if (HelpText == NULL) { > > - Status = 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 list > > of > > @@ -452,111 +320,6 @@ ManFileFindSections( > > return (Status); > > } > > > > -/** > > - parses through the MAN file formatted Buffer and returns the > > - "Brief Description" for the .TH section as specified by Command. If the > > - command section is not found return EFI_NOT_FOUND. > > - > > - Upon a sucessful return the caller is responsible to free the memory in > *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 > > description > 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 description > 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[] = L".TH "; > > - CONST CHAR16 EndString[] = L" 0 "; > > - > > - if ( Buffer == NULL > > - || Command == NULL > > - || (BriefDesc != NULL && BriefSize == NULL) > > - ){ > > - return (EFI_INVALID_PARAMETER); > > - } > > - > > - Status = EFI_SUCCESS; > > - > > - // > > - // Do not pass any leading path information that may be present to > IsTitleHeader(). > > - // > > - Start = StrLen(Command); > > - while ((Start != 0) > > - && (*(Command + Start - 1) != L'\\') > > - && (*(Command + Start - 1) != L'/') > > - && (*(Command + Start - 1) != L':')) { > > - --Start; > > - } > > - > > - // > > - // more characters for StartString and EndString > > - // > > - TitleLength = StrSize(Command + Start) + (StrLen(StartString) + > StrLen(EndString)) * sizeof(CHAR16); > > - TitleString = AllocateZeroPool(TitleLength); > > - if (TitleString == 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 = StrStr(*Buffer, TitleString); > > - if (CurrentLocation == NULL){ > > - Status = 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 += StrLen(TitleString) > > - ; *CurrentLocation == L' ' || *CurrentLocation == L'0' || > *CurrentLocation == L'1' || *CurrentLocation == L'\"' > > - ; CurrentLocation++); > > - > > - TitleEnd = StrStr(CurrentLocation, L"\""); > > - if (TitleEnd == NULL) { > > - Status = EFI_DEVICE_ERROR; > > - } else { > > - if (BriefDesc != NULL) { > > - *BriefSize = StrSize(TitleEnd); > > - *BriefDesc = AllocateZeroPool(*BriefSize); > > - if (*BriefDesc == NULL) { > > - Status = EFI_OUT_OF_RESOURCES; > > - } else { > > - StrnCpyS(*BriefDesc, (*BriefSize)/sizeof(CHAR16), > > CurrentLocation, > TitleEnd-CurrentLocation); > > - } > > - } > > - > > - for (CurrentLocation = TitleEnd > > - ; *CurrentLocation != L'\n' > > - ; CurrentLocation++); > > - for ( > > - ; *CurrentLocation == L' ' || *CurrentLocation == L'\n' || > *CurrentLocation == L'\r' > > - ; CurrentLocation++); > > - *Buffer = 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 > > return 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 = NULL; > > CmdFilePathName = NULL; > > CmdFileImgHandle = NULL; > > - StringBuff = NULL; > > PackageListHeader = NULL; > > FileDevPath = NULL; > > DevPath = NULL; > > @@ -846,11 +606,17 @@ ProcessManFile( > > // > > TempString = ShellCommandGetCommandHelp(Command); > > if (TempString != NULL) { > > - TempString2 = TempString; > > - Status = ManBufferFindTitleSection(&TempString2, Command, > BriefDesc, &BriefSize); > > + FileHandle = ConvertEfiFileProtocolToShellHandle > (CreateFileInterfaceMem (TRUE), NULL); > > + HelpSize = StrLen (TempString) * sizeof (CHAR16); > > + ShellWriteFile (FileHandle, &HelpSize, TempString); > > + ShellSetFilePosition (FileHandle, 0); > > + HelpSize = 0; > > + BriefSize = 0; > > + Status = ManFileFindTitleSection(FileHandle, Command, BriefDesc, > &BriefSize, &Ascii); > > if (!EFI_ERROR(Status) && HelpText != NULL){ > > - Status = ManBufferFindSections(TempString2, Sections, HelpText, > &HelpSize); > > + Status = ManFileFindSections(FileHandle, Sections, HelpText, > &HelpSize, Ascii); > > } > > + ShellCloseFile (&FileHandle); > > } else { > > // > > // If the image is a external app, check .MAN file first. > > @@ -947,20 +713,26 @@ ProcessManFile( > > > > StringIdWalker = 1; > > do { > > - SHELL_FREE_NON_NULL(StringBuff); > > + SHELL_FREE_NON_NULL(TempString); > > if (BriefDesc != NULL) { > > SHELL_FREE_NON_NULL(*BriefDesc); > > } > > - StringBuff = HiiGetString (mShellManHiiHandle, > (EFI_STRING_ID)StringIdWalker, NULL); > > - if (StringBuff == NULL) { > > + TempString = HiiGetString (mShellManHiiHandle, > (EFI_STRING_ID)StringIdWalker, NULL); > > + if (TempString == NULL) { > > Status = EFI_NOT_FOUND; > > goto Done; > > } > > - TempString2 = StringBuff; > > - Status = ManBufferFindTitleSection(&TempString2, Command, > BriefDesc, &BriefSize); > > + FileHandle = ConvertEfiFileProtocolToShellHandle > (CreateFileInterfaceMem (TRUE), NULL); > > + HelpSize = StrLen (TempString) * sizeof (CHAR16); > > + ShellWriteFile (FileHandle, &HelpSize, TempString); > > + ShellSetFilePosition (FileHandle, 0); > > + HelpSize = 0; > > + BriefSize = 0; > > + Status = ManFileFindTitleSection(FileHandle, Command, BriefDesc, > &BriefSize, &Ascii); > > if (!EFI_ERROR(Status) && HelpText != NULL){ > > - Status = ManBufferFindSections(TempString2, Sections, HelpText, > &HelpSize); > > + Status = ManFileFindSections(FileHandle, Sections, HelpText, > &HelpSize, Ascii); > > } > > + ShellCloseFile (&FileHandle); > > if (!EFI_ERROR(Status)){ > > // > > // Found what we need and return > > @@ -969,7 +741,7 @@ ProcessManFile( > > } > > > > StringIdWalker += 1; > > - } while (StringIdWalker < 0xFFFF && StringBuff != NULL); > > + } while (StringIdWalker < 0xFFFF && TempString != NULL); > > > > } > > > > @@ -992,7 +764,6 @@ Done: > > Status = gBS->UnloadImage (CmdFileImgHandle); > > } > > > > - SHELL_FREE_NON_NULL(StringBuff); > > SHELL_FREE_NON_NULL(TempString); > > SHELL_FREE_NON_NULL(CmdFileName); > > SHELL_FREE_NON_NULL(CmdFilePathName); > > > > > -- > Thanks, > Ray _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
