public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [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 command"
@ 2018-02-11 15:17 Ruiyu Ni
  2018-02-11 15:20 ` Ni, Ruiyu
  0 siblings, 1 reply; 3+ messages in thread
From: Ruiyu Ni @ 2018-02-11 15:17 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jaben Carsey

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 <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
---
 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);
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* 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 command"
  2018-02-11 15:17 [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 command" Ruiyu Ni
@ 2018-02-11 15:20 ` Ni, Ruiyu
  2018-02-12 15:23   ` [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 Carsey, Jaben
  0 siblings, 1 reply; 3+ messages in thread
From: Ni, Ruiyu @ 2018-02-11 15:20 UTC (permalink / raw)
  To: Carsey, Jaben, edk2-devel@lists.01.org

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 <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> ---
>   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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* 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 "descript...
  2018-02-11 15:20 ` Ni, Ruiyu
@ 2018-02-12 15:23   ` Carsey, Jaben
  0 siblings, 0 replies; 3+ messages in thread
From: Carsey, Jaben @ 2018-02-12 15:23 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org

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 <jaben.carsey@intel.com>; edk2-devel@lists.01.org
> 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 <ruiyu.ni@intel.com>
> > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > ---
> >   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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-02-12 15:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-11 15:17 [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 command" Ruiyu Ni
2018-02-11 15:20 ` Ni, Ruiyu
2018-02-12 15:23   ` [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 Carsey, Jaben

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox