Fix shadow global declaration on old versions of gcc

[ribes96]

Old versions of gcc (4.3.3 at least) complain on local variables shadowing global names. Some of them are well-known unix function names like 'send' and 'dup'. Others are simply local collisions.

Simply rename the local variables to avoid the clash.

Description

Please describe the scope of the fix or feature addition.

Simply rename of variables to avoid name collision

Fixes zd#

Testing

How did you test?

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

[wolfSSL-Bot]

Can one of the admins verify this patch?

[dgarske]

Hi @ribes96 ,

Thank you for this pull request. This issue with shadow variable names comes up peridically. It reminds me that we need a build test case for this. Can you tell us more about your project and use case for wolfSSL? In order to accept third-party code/PR's we need to get you setup as a contributor. Please email support at wolfssl dot com and reference this PR. Please include your contact information including your location.

Thanks,
David Garske, wolfSSL

[douzzer]

Hi @ribes96 -- thanks for the contribution! Upon further investigation of the issue, we're inclined to recommend addressing this problem by passing -Wno-shadow to your compiler. Warnings on those code patterns are considered harmful (see for example https://lkml.org/lkml/2006/11/28/239), and modern gcc has no mechanism to enable them. gcc seems to have changed the semantics of -Wshadow in release 4.8 (march 2013), and clang and indeed clang-tidy similarly have no mechanism to warn on declarations shadowing functions.

If -Wno-shadow doesn't resolve the warnings for you, feel free to reopen this ticket and we'll investigate further.

[ribes96]

Passing -Wno-shadow was the first thing I tried. Turns out the build system places -Wshadow at the end of CFLAGS, overriding the previous -Wno-shadow flag. I was building with autotools.

[douzzer]

@ribes96 note that I have a PR in process that addresses all of the name conflicts you identified, except for the CipherType change in wolfcrypt/src/evp.c. That doesn't appear to be a shadowing issue -- can you explain it further?

[ribes96]

At line 326 of file wolfcrypt/src/evp.c there is a function declaration with that name

static unsigned int cipherType(const WOLFSSL_EVP_CIPHER *cipher);

The definition is at line 1850

  static unsigned int cipherType(const WOLFSSL_EVP_CIPHER *cipher)                
  {                                                                               
      if (cipher == NULL) return 0; /* dummy for #ifdef */                        
  #ifndef NO_DES3                                                                 
      else if (EVP_CIPHER_TYPE_MATCHES(cipher, EVP_DES_CBC))                      
          return WC_DES_CBC_TYPE;                                                 
      else if (EVP_CIPHER_TYPE_MATCHES(cipher, EVP_DES_EDE3_CBC))                 
          return WC_DES_EDE3_CBC_TYPE;                                            
  #if !defined(NO_DES3)                                                           
      else if (EVP_CIPHER_TYPE_MATCHES(cipher, EVP_DES_ECB))                      
          return WC_DES_ECB_TYPE;                                                 
      else if (EVP_CIPHER_TYPE_MATCHES(cipher, EVP_DES_EDE3_ECB))                 
          return WC_DES_EDE3_ECB_TYPE;                                            
  #endif /* NO_DES3 && HAVE_AES_ECB */                                            
  #endif 

Then at line 8262 there is a local variable also named cipherType

    static int IsCipherTypeAEAD(unsigned char cipherType)
    {
        switch (cipherType) {
            case WC_AES_128_GCM_TYPE:
            case WC_AES_192_GCM_TYPE:
            case WC_AES_256_GCM_TYPE:
            case WC_AES_128_CCM_TYPE:        
            case WC_AES_192_CCM_TYPE:
            case WC_AES_256_CCM_TYPE:
            case WC_ARIA_128_GCM_TYPE:
            case WC_ARIA_192_GCM_TYPE:
            case WC_ARIA_256_GCM_TYPE:
            case WC_SM4_GCM_TYPE:
            case WC_SM4_CCM_TYPE:
                return 1;
            default:
                return 0;
        }
    }

[douzzer]

@ribes96 Ah, well this will not do -- an enum, a function, and a local declaration as an unsigned char. Definitely too much -- will orthogonalize the names in a followup PR.

[ribes96]

Do you consider the fact that autotools overrides CFLAGS with options at the end a bug? Should I fill a bug report? Specially considering that shadowed variables could creep in again in the future...

I was unable to find a way to tell autotools to place my -Wno-shadow at the end.