Re: [PATCH edk2-platforms 5/5] Platform/RDKQemu: stop using deprecated string conversion routines

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Mon, Jun 10, 2019 at 04:20:06PM +0200, Ard Biesheuvel wrote:
> Stop using deprecated string conversion routines so we can stop
> un'#define'ing the DISABLE_NEW_DEPRECATED_INTERFACES macro in this code.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/Comcast/Library/RdkBootManagerLib/DiskIo.c  | 12 +++++++--
>  Platform/Comcast/Library/RdkBootManagerLib/RdkFile.c | 28 +++++++++++---------
>  Platform/Comcast/RDKQemu/RDKQemu.dsc                 |  3 ---
>  3 files changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/Platform/Comcast/Library/RdkBootManagerLib/DiskIo.c b/Platform/Comcast/Library/RdkBootManagerLib/DiskIo.c
> index ed893bd5af6a..df16c326cc57 100644
> --- a/Platform/Comcast/Library/RdkBootManagerLib/DiskIo.c
> +++ b/Platform/Comcast/Library/RdkBootManagerLib/DiskIo.c
> @@ -90,6 +90,7 @@ ListBlockIos (
>    UINTN                             NumHandles;
>    UINT16                            *DeviceFullPath;
>    DISKIO_PARTITION_LIST             *Entry;
> +  RETURN_STATUS                     RetStatus;
>  
>    InitializeListHead (&mPartitionListHead);
>  
> @@ -146,11 +147,13 @@ ListBlockIos (
>  
>        // Copy handle and partition name
>        Entry->PartitionHandle = AllHandles[LoopIndex];
> -      StrnCpy (
> +      RetStatus = StrnCpyS (
>          Entry->PartitionName,
> +        PARTITION_NAME_MAX_LENGTH,
>          PartitionName,
>          PARTITION_NAME_MAX_LENGTH
>        );
> +      ASSERT_RETURN_ERROR (RetStatus);

Would we not want to return an error here, for non-DEBUG builds?

>        InsertTailList (&mPartitionListHead, &Entry->Link);
>        break;
>      }
> @@ -176,8 +179,13 @@ OpenPartition (
>    DISKIO_PARTITION_LIST    *Entry;
>    SPARSE_HEADER            *SparseHeader;
>    UINT16                   UnicodePartitionName[100];
> +  RETURN_STATUS            RetStatus;
>  
> -  AsciiStrToUnicodeStr ( PartitionName, UnicodePartitionName);
> +  RetStatus = AsciiStrToUnicodeStrS (PartitionName, UnicodePartitionName,
> +                sizeof (UnicodePartitionName));
> +  if (RETURN_ERROR (RetStatus)) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
>    DEBUG((DEBUG_INFO, "Unicode partition name %s\n", UnicodePartitionName));
>  
>    Status = ListBlockIos (UnicodePartitionName);
> diff --git a/Platform/Comcast/Library/RdkBootManagerLib/RdkFile.c b/Platform/Comcast/Library/RdkBootManagerLib/RdkFile.c
> index dd695651026a..6f98562eff84 100644
> --- a/Platform/Comcast/Library/RdkBootManagerLib/RdkFile.c
> +++ b/Platform/Comcast/Library/RdkBootManagerLib/RdkFile.c
> @@ -8,8 +8,6 @@
>  
>  #define MAX_VAR       6
>  
> -#define ALLOCATE_STRING_MEM(X)  AllocateZeroPool((X + 1) * sizeof(CHAR16))
> -
>  /**
>   * list_for_each_entry	-	iterate over list of given type
>   * @pos:	the type * to use as a loop cursor.
> @@ -53,10 +51,13 @@ SaveString (
>    IN  CHAR16    *String2
>    )
>  {
> -  *Dest = ALLOCATE_STRING_MEM (StrLen (String1) + StrLen (String2));
> +  UINTN StringLength;
> +
> +  StringLength = StrLen (String1) + StrLen (String2) + 1;
> +  *Dest = AllocatePool (StringLength * sizeof(CHAR16));

Minor, and just part of the shuffle, but - space after sizeof?
(At least one more below.)

/
    Leif

>    ASSERT (Dest != NULL);
> -  StrCat (*Dest, String1);
> -  StrCat (*Dest, String2);
> +  StrCpyS (*Dest, StringLength, String1);
> +  StrCatS (*Dest, StringLength, String2);
>  }
>  
>  STATIC
> @@ -74,11 +75,13 @@ LsFiles (
>    BOOLEAN             NoFile;
>    CHAR16              *TempPath;
>    DIR_NODE            *Node;
> +  UINTN               StringLength;
>  
>    NoFile    = FALSE;
> -  TempPath  = ALLOCATE_STRING_MEM (StrLen(DirPath) + 1);
> -  StrCat (TempPath, DirPath);
> -  StrCat (TempPath, L"/");
> +  StringLength = StrLen(DirPath) + 2;
> +  TempPath = AllocatePool (StringLength * sizeof(CHAR16));
> +  StrCpyS (TempPath, StringLength, DirPath);
> +  StrCatS (TempPath, StringLength, L"/");
>  
>    Status = GetFileHandler (&FileHandle, DirPath, EFI_FILE_MODE_READ);
>    ASSERT_EFI_ERROR (Status);
> @@ -192,6 +195,7 @@ InitVarList (
>    UINTN       Next;
>    CHAR8       *VarDelimiter[2];
>    EFI_STATUS  Status;
> +  UINTN       StringLength;
>  
>    VarDelimiter[0] = "=";
>    VarDelimiter[1] = "\"";
> @@ -212,10 +216,10 @@ InitVarList (
>        if (VarResult[OuterLoopIndex][InnerLoopIndex]) {
>          FreePool (VarResult[OuterLoopIndex][InnerLoopIndex]);
>        }
> -      VarResult[OuterLoopIndex][InnerLoopIndex] = \
> -        ALLOCATE_STRING_MEM (AsciiStrLen (&FileData[Current]));
> -      AsciiStrToUnicodeStr (&FileData[Current], \
> -        VarResult[OuterLoopIndex][InnerLoopIndex]);
> +      StringLength = AsciiStrLen (&FileData[Current]) + 1;
> +      VarResult[OuterLoopIndex][InnerLoopIndex] = AllocatePool (StringLength);
> +      AsciiStrToUnicodeStrS (&FileData[Current],
> +        VarResult[OuterLoopIndex][InnerLoopIndex], StringLength);
>        //skip new line
>        Next += 2;
>      }
> diff --git a/Platform/Comcast/RDKQemu/RDKQemu.dsc b/Platform/Comcast/RDKQemu/RDKQemu.dsc
> index f0ed4f11e81d..440d2ace917c 100644
> --- a/Platform/Comcast/RDKQemu/RDKQemu.dsc
> +++ b/Platform/Comcast/RDKQemu/RDKQemu.dsc
> @@ -418,6 +418,3 @@ [Components.AARCH64]
>      <LibraryClasses>
>        NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>    }
> -
> -[BuildOptions]
> -  GCC:*_*_*_CC_FLAGS = -UDISABLE_NEW_DEPRECATED_INTERFACES
> -- 
> 2.20.1
>