Coverity issues involving negative arguments

[paulidale]

• documentation is added or updated
• tests are added or updated

[t8m]

Was this closed intentionally? Although maybe we should just change those _size() calls to return 0 instead of negative values.

[paulidale]

No, I must have closed it by accident. Lots of these outstanding at present.

[t8m]

I really wonder if introducing the negative return value for these calls was right.

[paulidale]

These are mostly historic calls, so we have to live with them.
A new call that did something like this would be different. Then again, a new call that returns a size as an int is suspect from the start.

[paulidale]

ping for review

[paulidale]

Still feeling unrequited review.

[t8m]

I'm still a little bit worried for the cases where we are newly introducing the negative error return value in 3.0. This can have quite serious impact in applications if they aren't properly ready to handle negative error returns.

[paulidale]

This isn't introducing any such functions. I don't think it's checking the returns from any either.

DH_size(), EVP_CIPHER_CTX_key_length() and EVP_CIPHER_CTX_iv_length() are all pre-existing.

I agree, it's a concern and we really shouldn't be doing it.

[t8m]

DH_size(), EVP_CIPHER_CTX_key_length() and EVP_CIPHER_CTX_iv_length() are all pre-existing.

I do not think DH_size() and EVP_CIPHER_CTX_key_length() can ever return < 0 values in 1.1.1.
EVP_CIPHER_CTX_iv_length() is pre-existing.

I am not saying this PR is introducing it. I am saying 3.0 introduces it. So maybe the right fix for this PR is to actually drop the negative returns for these.

Okay I understand now. DH_size() hasn't changed between versions. I've not dug through the EVP_CIPHER_CTX_key_length() paths yet.

[paulidale]

In essence, you are suggesting returning zero from EVP_CIPHER_CTX_key_length() instead of the EVP_CTRL_RET_UNSUPPORTED value. I'm not convinced that this is a good idea. 0 or negative returns are both going to cause problems.

All we're saving are the programs that check == 0 instead of <= 0.

Our documentation isn't forgiving here: negative and zero returns are not mentioned.

[FdaSilvaYY]

Ping @openssl ; This PR is falling into the limbo.

[paulidale]

Rebased to latest master, still waiting for a review or a suggestion about the negative return changes required.

[mattcaswell]

This looks fine to me. This PR isn't introducing any changes which causes functions to return <0 where they didn't before. So, if that has occurred it is out of scope of this PR to fix it IMO.

[openssl-machine]

This pull request is ready to merge

[paulidale]

Merged to master, thanks for the comments etc.