aws / s2n-tls

An implementation of the TLS/SSL protocols
https://aws.github.io/s2n-tls/usage-guide/
Apache License 2.0
4.52k stars 704 forks source link

ci: check for s2n_array_len in loop bounds #4802

Closed lrstewart closed 2 days ago

lrstewart commented 1 week ago

Description of changes:

Whenever I'm writing a test of the form:

struct {
    /* some variable arguments */
} test_cases[] = { /* some test cases */ };
for (size_t i = 0; i < s2n_array_len(test_cases); i++) {

I'm always worried that I'm going to do "sizeof" instead of "s2n_array_len". Best case, that would cause the test to iterate over test cases that don't exist, probably failing, at a minimum probably accessing invalid memory. But worst case, the test silently doesn't execute some test cases. I've made this mistake before.

So I added a grep_simple_mistakes that only allows sizeof in loops for variables with names indicating that's probably safe. I'd love to try to expand it to all uses of sizeof, but that's a MUCH larger set of mistakes. Loops are also what I'm most concerned about: it's a common pattern, and it's very hard to detect the mistake. Other sizeofs are generally used in assertions.

Testing:

Tests continue to pass. Here's an example of a violation:

Warning: sizeof is only valid for arrays of chars or uint8_ts. Use s2n_array_len for other types, or append "bytes", "data", or "u8" to your variable name for clarity. Warning in /home/lrstewart/Code/s2n-tls/tests/unit/s2n_resume_test.c:
408:        for (size_t i = 0; i < sizeof(ems_state); i++) {

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