Re: [PATCH 3/7] OvmfPkg/PlatformPei: use QemuFwCfgParseBool in UPDATE_BOOLEAN_PCD_FROM_...

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/24/20 9:53 AM, Laszlo Ersek wrote:
> The UPDATE_BOOLEAN_PCD_FROM_FW_CFG() macro currently calls the
> module-private helper function GetNamedFwCfgBoolean(). Replace the latter
> with QemuFwCfgParseBool() from QemuFwCfgSimpleParserLib.
> 
> This change is compatible with valid strings accepted previously.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Per Sundstrom <per_sundstrom@yahoo.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2681
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   OvmfPkg/PlatformPei/Platform.c | 47 +-------------------
>   1 file changed, 2 insertions(+), 45 deletions(-)
> 
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 088e616a980c..3b850c2c2626 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -27,6 +27,7 @@
>   #include <Library/PeiServicesLib.h>
>   #include <Library/QemuFwCfgLib.h>
>   #include <Library/QemuFwCfgS3Lib.h>
> +#include <Library/QemuFwCfgSimpleParserLib.h>
>   #include <Library/ResourcePublicationLib.h>
>   #include <Ppi/MasterBootMode.h>
>   #include <IndustryStandard/I440FxPiix4.h>
> @@ -254,56 +255,12 @@ MemMapInitialization (
>     ASSERT_RETURN_ERROR (PcdStatus);
>   }
>   
> -EFI_STATUS
> -GetNamedFwCfgBoolean (
> -  IN  CHAR8   *FwCfgFileName,
> -  OUT BOOLEAN *Setting
> -  )
> -{
> -  EFI_STATUS           Status;
> -  FIRMWARE_CONFIG_ITEM FwCfgItem;
> -  UINTN                FwCfgSize;
> -  UINT8                Value[3];
> -
> -  Status = QemuFwCfgFindFile (FwCfgFileName, &FwCfgItem, &FwCfgSize);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -  if (FwCfgSize > sizeof Value) {
> -    return EFI_BAD_BUFFER_SIZE;
> -  }
> -  QemuFwCfgSelectItem (FwCfgItem);
> -  QemuFwCfgReadBytes (FwCfgSize, Value);

Until here QemuFwCfgGetAsString(), OK.

> -
> -  if ((FwCfgSize == 1) ||
> -      (FwCfgSize == 2 && Value[1] == '\n') ||
> -      (FwCfgSize == 3 && Value[1] == '\r' && Value[2] == '\n')) {

StripNewline(), OK.

> -    switch (Value[0]) {
> -      case '0':
> -      case 'n':
> -      case 'N':
> -        *Setting = FALSE;

mFalseString[], OK.

(And we get disable[d] + false).

> -        return EFI_SUCCESS;
> -
> -      case '1':
> -      case 'y':
> -      case 'Y':
> -        *Setting = TRUE;

mTrueString[], OK.

(And we get enable[d] + true).

> -        return EFI_SUCCESS;
> -
> -      default:
> -        break;
> -    }
> -  }
> -  return EFI_PROTOCOL_ERROR;
> -}
> -
>   #define UPDATE_BOOLEAN_PCD_FROM_FW_CFG(TokenName)                   \
>             do {                                                      \
>               BOOLEAN       Setting;                                  \
>               RETURN_STATUS PcdStatus;                                \
>                                                                       \
> -            if (!EFI_ERROR (GetNamedFwCfgBoolean (                  \
> +            if (!RETURN_ERROR (QemuFwCfgParseBool (                 \

:)

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

>                                 "opt/ovmf/" #TokenName, &Setting))) { \
>                 PcdStatus = PcdSetBoolS (TokenName, Setting);         \
>                 ASSERT_RETURN_ERROR (PcdStatus);                      \
>