[openssl-dev] [openssl.org #3862] Patch: Compiler messages on OpenVMS

[Matt Caswell via RT]

On Mon May 25 22:02:21 2015, zinser at zinser.no-ip.info wrote:
> Hello,
>
> I've just build OpenSSL 1.0.2a on
>
> OpenVMS 8.3 (Alpha)
> HP C V 7.3
>
> The build (and test) went very smooth, with just three minor
> complaints by the
> compiler:
>
> Compiling The BIO Library Files. (LIBRARY,LIB)
> bio_lib.c
> bio_cb.c
> ...
> bss_dgram.c
>
> if (timeleft.tv_sec < 0) {
> ............^
> %CC-I-QUESTCOMPARE, In this statement, the unsigned expression
> "timeleft.tv_sec" is being compared with a relational operator to a
> constant
> whose v
> alue is not greater than zero. This might not be what you intended.
> at line number 313 in file PUBLIC$ROOT:
> [UTIL.LIBS.OPENSSL102.CRYPTO.BIO]BSS_DGRAM.C;1
>
> ...
> Building The SYS$DISK:[-.ALPHA.EXE.SSL]SSL_LIBSSL32.OLB Library.
> s2_meth.c
> s2_srvr.c
> ...
> t1_lib.c
>
> if (size <= 0 || ((len = data[0])) != (size - 1)) {
> ................^
> %CC-I-QUESTCOMPARE, In this statement, the unsigned expression "size"
> is being
> compared with a relational operator to a constant whose value is not
> greater than zero. This might not be what you intended.
> at line number 2085 in file
> PUBLIC$ROOT:[UTIL.LIBS.OPENSSL102.SSL]T1_LIB.C;1
>
> ...
> Compiling The ccgost Library Files. (ENGINES)
> e_gost_err
> gost2001_keyx
> gost2001
> gost89
> gost94_keyx
>
> if (*outlen <= 0) {
> ........^
> %CC-I-QUESTCOMPARE, In this statement, the unsigned expression
> "*outlen" is
> being compared with a relational operator to a constant whose value is
> not greater than zero. This might not be what you intended.
> at line number 178 in file PUBLIC$ROOT:
> [UTIL.LIBS.OPENSSL102.ENGINES.CCGOST]GOST94_KEYX.C;1
>
>
> As can be easily seen, all three complaints are of the same type. They
> also
> can be easily cured by explicitly casting the the questionable
> parameters to
> (signed) int.

Casting will make the warnings go away but doesn't solve the underlying issues
- it just hides them.

Taking each of the three warnings in turn:

1) It seems the tv_sec element of struct timeval is unsigned on VMS whilst on
most other platforms it is signed. The logic used to calculate timeouts takes
one time away from the other and then tests to see if the result is negative.
This is likely to cause problems on VMS. I think the correct solution here is
to rewrite the timeout logic to not rely on negative values (which is a
straight forward change).

2) In the second issue the size of the SRP extension is checked to see if it is
<= 0. This value is extracted from the incoming packet and can never be
negative and indeed is held in an unsigned variable. Testing for < 0 is
nonsensical. Therefore the correct change here is just to check |size == 0|.

3) The GOST engine issue is in the handling of the return value from an "i2d"
function. These functions can return a negative value on error. However the
GOST engine is storing the result in an *unsigned* variable and *then* testing
if it is negative. This is obviously going to cause problems if that function
ever fails. I think the correct solution here is to store the result in a
temporary signed int first and then handle any error conditions.

I have pushed fixes for all of the above.

Thanks for your report,

Matt