Enable -Wsign-Compare-check_bin/_crypto/_stuffer/_utils/

[aditishri18]

Resolved issues:

related to : #3697

Description of changes:

This PR is a part of series of PR's to enable -Wsign-compare check. This check warns when a comparison between signed and unsigned values produce an incorrect result. Warning shown when the signed value is converted to unsigned value.

This PR contains changes made to files in bin/ , crypto/ , stuffer/ and utils/.

Call-outs:

To be consistent, I changed the data type int to size_t in for loops and where sizeof operator is used.

Testing:

All tests pass.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[camshaft]

In general, we need to be really careful about conversions that

• go from signed to unsigned (e.g. int -> uint32_t)
• go from a large-sized integer to a smaller-sized integer (e.g. uint32_t -> uint16_t)

[aditishri18]

What would be a scenario where downcast (e.g. uint32_t -> uint16_t) is required ?

[camshaft]

What would be a scenario where downcast (e.g. uint32_t -> uint16_t) is required ?

If you have an integer of two sizes you always want to cast to the largest-sized integer that will contain both sides of the comparison. For example, if I want to check if a int and uint32_t are equal, I'd need a int64_t since neither type could hold each others min/max values. For example, you had this before:

s2n-tls/crypto/s2n_cbc_cipher_3des.c

Line 38 in 09d90b6

S2N_ERROR_IF(len != (int) in->size, S2N_ERR_ENCRYPT);

This means that if in->size is greater than the max value of int (2,147,483,647), then size will wrap and become negative and you can possibly get nonsensical results.

In other words, try to avoid going to a lower bitsize, especially if you haven't checked that the value can actually fit in the lower value. Or just always cast up to the smallest bitsize that will fit the min and max from both operand types.

[aditishri18]

That explains a lot, thanks.

[camshaft]

sorry one more nit