Re: [PATCH v2 06/11] CryptoPkg: Fix handling of &strcmp function pointers

[Laszlo Ersek <lersek@redhat.com>]

On 03/23/17 14:19, Qin Long wrote:
> In a couple of places, OpenSSL code uses the address of the
> strcmp() function, and assigns it to another comparator function
> pointer.
> 
> Unfortunately, this falls foul of the inconsistent function ABI
> that we use in EDKII. We '#define strcmp AsciiStrCmp' but AsciiStrCmp
> is an EFIAPI function with the Microsoft ABI. And we're assigning its
> address to a non-EFIAPI function, which may well have a different ABI.
> 
> Fix this by providing an actual strcmp() function in the default ABI.
> We already *had* a prototype for it in OpenSslSupport.h, which was
> then superseded by the #define strcmp AsciiStrCmp.
> 
> Now, OpenSSL code *can* use &strcmp without problems.
> 
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Lin <glin@suse.com>
> Cc: Ronald Cron <ronald.cron@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Qin Long <qin.long@intel.com>
> ---
>  CryptoPkg/Include/OpenSslSupport.h                | 3 +--
>  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c | 8 +++++++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/CryptoPkg/Include/OpenSslSupport.h b/CryptoPkg/Include/OpenSslSupport.h
> index e011a7cfee..e6858a94a4 100644
> --- a/CryptoPkg/Include/OpenSslSupport.h
> +++ b/CryptoPkg/Include/OpenSslSupport.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Root include file to support building OpenSSL Crypto Library.
>  
> -Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD License
>  which accompanies this distribution.  The full text of the license may be found at
> @@ -276,7 +276,6 @@ extern FILE  *stdout;
>  #define memchr(buf,ch,count)              ScanMem8(buf,(UINTN)(count),(UINT8)ch)
>  #define memcmp(buf1,buf2,count)           (int)(CompareMem(buf1,buf2,(UINTN)(count)))
>  #define memmove(dest,source,count)        CopyMem(dest,source,(UINTN)(count))
> -#define strcmp                            AsciiStrCmp
>  #define strncmp(string1,string2,count)    (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
>  #define strcpy(strDest,strSource)         AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource)
>  #define strncpy(strDest,strSource,count)  AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
> diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> index 9d6867ebce..e8a76d07ff 100644
> --- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> +++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> @@ -2,7 +2,7 @@
>    Intrinsic Memory Routines Wrapper Implementation for OpenSSL-based
>    Cryptographic Library.
>  
> -Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD License
>  which accompanies this distribution.  The full text of the license may be found at
> @@ -15,6 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  
>  #include <Base.h>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/BaseLib.h>
>  
>  /* OpenSSL will use floating point support, and C compiler produces the _fltused
>     symbol by default. Simply define this symbol here to satisfy the linker. */
> @@ -44,3 +45,8 @@ void * memset (void *dest, char ch, unsigned int count)
>    
>    return dest;
>  }
> +
> +int strcmp (const char *s1, const char *s2)
> +{
> +  return (int)AsciiStrCmp(s1, s2);
> +}
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

(Tested as described in
<http://mid.mail-archive.com/4410fa53-b0e4-d64a-7b95-8a430a4c7b06@redhat.com>.)

Thanks!
Laszlo