review windows warnings

[chipitsine]

while working on automatic release, I noticed a lot of build warnings under windows.
let's review them.

either we can

1. leave as is
2. fix them and make fatal
3. mute them

   crypto_init.c
D:\a\portable\portable\crypto\..\include\compat\pthread.h(36,1): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\crypto_init.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\..\include\compat\pthread.h(36,1): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\cryptlib.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\..\include\compat\pthread.h(44,71): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\crypto_init.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\..\include\compat\pthread.h(44,71): warning C4152: nonstandard extension, function/data pointer conversion in expression (compiling source file D:\a\portable\portable\crypto\cryptlib.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
  cversion.c

[chipitsine]

one more like that

D:\a\portable\portable\crypto\x509\x509_vpm.c(98,8): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\x509\x509_vpm.c(99,7): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\x509\x509_vpm.c(316,49): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\x509\x509_vpm.c(316,59): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

[chipitsine]

... and more

D:\a\portable\portable\tests\x509_asn1.c(36,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(37,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(38,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(39,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(40,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(41,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(42,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(43,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(44,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(45,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(46,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(47,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(48,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(95,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(111,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(124,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(185,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]
D:\a\portable\portable\tests\x509_asn1.c(198,19): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]

[botovq]

Yes, thanks. I saw them

D:\a\portable\portable\crypto\x509\x509_vpm.c(98,8): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

sk_deep_copy() should just use the proper function pointers passed in its only caller instead of being written in this pseudo generality. But that needs to be done in the OpenBSD repo.

D:\a\portable\portable\tests\x509_asn1.c(36,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]

this test needs reworking. I've added it to my to do list.

[botovq]

• static const not necessarily ininitialized to 0?

  D:\a\portable\portable\crypto\evp\e_chacha20poly1305.c(107,43): warning C4132: 'zero_pad16': const object should be initialized [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

  unsure about this one. I would have expected this to be fine.

  Added: It was confirmed to me that tihs is ok as it is (according to C2x 6.7.9, clause 10), so a false positive. We won't work around htis.

• freeing of const object

  D:\a\portable\portable\crypto\objects\o_names.c(317,15): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

  This code will hopefully be replaced soon.

• potential use of uninitialized

  D:\a\portable\portable\crypto\pem\pem_lib.c(424): warning C4701: potentially uninitialized local variable 'j' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
  D:\a\portable\portable\crypto\pem\pem_lib.c(475): warning C4701: potentially uninitialized local variable 'i' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

  This looks like a correct warning and should be fixed. It is actually a false positive. i and j will only be used when they were initialized. The code is horrible and should be reworked. This will likely happen as part of a related cleanup soon.

• Unreachable code

  D:\a\portable\portable\crypto\pkcs7\pk7_doit.c(657): warning C4702: unreachable code [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

  Ugly code. Harmless.

• passing a function pointer through void

  D:\a\portable\portable\crypto\x509\x509_vpm.c(98,1): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

  As mentioned above, sk_deep_copy() should be fixed. I have a diff that I will send out for review soon.

• initializing an array with a longer string constant

  D:\a\portable\portable\crypto\compat\chacha_private.h(51,49): warning C4295: 'sigma': array is too small to include a terminating null character (compiling source file D:\a\portable\portable\crypto\compat\arc4random.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

  Already discussed. Correct C, reasonable warning.

• unreachable code

  D:\a\portable\portable\ssl\d1_pkt.c(1000): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
  D:\a\portable\portable\ssl\s3_cbc.c(464): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
  D:\a\portable\portable\ssl\s3_cbc.c(465): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
  D:\a\portable\portable\ssl\s3_cbc.c(466): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

  abuse of OPENSSL_assert(0).

• const issue with tlsext_build_order. Need to think about how I want to fix this. Harmless.

  D:\a\portable\portable\ssl\ssl_lib.c(571,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
  D:\a\portable\portable\ssl\ssl_tlsext.c(2256,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
  D:\a\portable\portable\ssl\ssl_tlsext.c(2305,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

• whining about the empty empty.c

  D:\a\portable\portable\crypto\empty.c(1,1): warning C4206: nonstandard extension used: translation unit is empty [D:\a\portable\portable\build\crypto\crypto.vcxproj]

  I have no idea what empty.c is about. It was introduced in 80eb145 but I don't understand what that is doing/fixing.

• invalid octal

  D:\a\portable\portable\tests\constraints.c(98,2): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\constraints.vcxproj]
  D:\a\portable\portable\tests\tlsexttest.c(4370,15): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\tlsexttest.vcxproj]

  this is nonsense. will fix.

• unsure what this is about

  D:\a\portable\portable\tests\freenull.c(214,2): warning C4115: 'lhash_st_FUNCTION': named type definition in parentheses [D:\a\portable\portable\build\tests\freenull.vcxproj]

  The offending line can probably be deleted.

• double const due to ugly typedef

  D:\a\portable\portable\tests\rfc3779.c(364,28): warning C4114: same type qualifier used more than once [D:\a\portable\portable\build\tests\rfc3779.vcxproj]

  typedef const ASN1_ITEM ASN1_ITEM_EXP... sigh.

• passing a const pointer through void

  D:\a\portable\portable\tests\x509_asn1.c(36,2): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\tests\x509_asn1.vcxproj]

[chipitsine]

D:\a\portable\portable\crypto\evp\e_chacha20poly1305.c(107,43): warning C4132: 'zero_pad16': const object should be initialized [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

unsure about this one. I would have expected this to be fine.

D:\a\portable\portable\crypto\objects\o_names.c(317,15): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

This code will hopefully be replaced soon.

D:\a\portable\portable\crypto\pem\pem_lib.c(424): warning C4701: potentially uninitialized local variable 'j' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]
D:\a\portable\portable\crypto\pem\pem_lib.c(475): warning C4701: potentially uninitialized local variable 'i' used [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

This looks like a correct warning and should be fixed.

D:\a\portable\portable\crypto\pkcs7\pk7_doit.c(657): warning C4702: unreachable code [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

Ugly code. Harmless.

D:\a\portable\portable\crypto\x509\x509_vpm.c(98,1): warning C4152: nonstandard extension, function/data pointer conversion in expression [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

As mentioned above, sk_deep_copy() should be fixed.

D:\a\portable\portable\crypto\compat\chacha_private.h(51,49): warning C4295: 'sigma': array is too small to include a terminating null character (compiling source file D:\a\portable\portable\crypto\compat\arc4random.c) [D:\a\portable\portable\build\crypto\crypto_obj.vcxproj]

Already discussed. Correct C, reasonable warning.

D:\a\portable\portable\ssl\d1_pkt.c(1000): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\s3_cbc.c(464): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\s3_cbc.c(465): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\s3_cbc.c(466): warning C4702: unreachable code [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

abuse of OPENSSL_assert(0).

D:\a\portable\portable\ssl\ssl_lib.c(571,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\ssl_tlsext.c(2256,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]
D:\a\portable\portable\ssl\ssl_tlsext.c(2305,28): warning C4090: 'function': different 'const' qualifiers [D:\a\portable\portable\build\ssl\ssl_obj.vcxproj]

const issue with tlsext_build_order. Need to think about how I want to fix this. Harmless.

D:\a\portable\portable\crypto\empty.c(1,1): warning C4206: nonstandard extension used: translation unit is empty [D:\a\portable\portable\build\crypto\crypto.vcxproj]

I have no idea what empty.c is about.

I've added build artifacts, entire build folder is added to artifacts

no idea what empty.c is either.
but we can have a look. it might be something automatically generated ...

D:\a\portable\portable\tests\constraints.c(98,2): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\constraints.vcxproj]
D:\a\portable\portable\tests\tlsexttest.c(4370,15): warning C4125: decimal digit terminates octal escape sequence [D:\a\portable\portable\build\tests\tlsexttest.vcxproj]

this is nonsense. will fix.

D:\a\portable\portable\tests\freenull.c(214,2): warning C4115: 'lhash_st_FUNCTION': named type definition in parentheses [D:\a\portable\portable\build\tests\freenull.vcxproj]

This should probably be deleted.

D:\a\portable\portable\tests\rfc3779.c(364,28): warning C4114: same type qualifier used more than once [D:\a\portable\portable\build\tests\rfc3779.vcxproj]

typedef const ASN1_ITEM ASN1_ITEM_EXP... sigh.

[vszakats]

To silence the chacha warnings, something like this can be done:

- static const char sigma[16] = "expand 32-byte k";
+ static const char sigma[16] = { 'e', 'x', 'p', 'a', 'n', 'd', ' ', '3',
+                                 '2', '-', 'b', 'y', 't', 'e', ' ', 'k' };

[botovq]

On Wed, Dec 13, 2023 at 08:19:24AM -0800, Viktor Szakats wrote: To silence the chacha warnings, something like this can be done: ```diff - static const char sigma[16] = "expand 32-byte k"; + static const char sigma[16] = { 'e', 'x', 'p', 'a', 'n', 'd', ' ', '3', + '2', '-', 'b', 'y', 't', 'e', ' ', 'k' }; ```

Yes, but that's also ugly and I don't think we should do this. See also #955

[chipitsine]

for ugly things we have "pragma" ))

[vszakats]

To me the ugly solution reduces the ambigiuty when reading the code. The string is just a bunch of bytes, not meant for reading or grepping. Another solution is to bump its size to [] or 17 and adjust any sizeof()s. What's against both is the diverging from the reference source. FWIW clang and gcc is silent about it.

As for this one:

D:\a\portable\portable\crypto\empty.c(1,1): warning C4206: nonstandard extension used: translation unit is empty

It'd be useful to know what exact CMake issue (and which Generator) made this necessary. Other than that, this warning may happen when the whole source content is guarded out legitimately, leaving an empty object. It think it's safe to silence, or ignore. If that's the only source affected, it'd be better to fix the CMake root issue though.