Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: make s2n_array_len constant #4789

Closed
wants to merge 2 commits into from
Closed

Conversation

lrstewart
Copy link
Contributor

Description of changes:

Our grep_simple_mistakes script requires that we use s2n_array_len everywhere instead of (sizeof(array) / sizeof(array[0])), but s2n_array_len is no longer defined as (sizeof(array) / sizeof(array[0])), so it's no longer constant. That means that you can't do like:

struct s2n_blob array1[100] = { 0 };
struct s2n_blob array2[s2n_array_len(array1)] = { 0 };

You could do (sizeof(array) / sizeof(struct s2n_blob)) as a workaround, but that's unnecessary and more brittle.

The null check is also unnecessary because arrays can't be null. Only pointers can be null, and s2n_array_len doesn't work on pointers anyway. You could argue that we need a sizeof(array) > 0 check instead, but funnily enough array[0] actually doesn't seg fault for an empty array: https://godbolt.org/z/s4vcrr6fP. I guess that makes sense, since you don't have to dereference anything. I'd hope at least some compilers or static analyzers would yell at you though.

Testing:

I'm just removing an unnecessary null check from a macro. I'm not sure how I'd test this.

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

@github-actions github-actions bot added the s2n-core team label Sep 20, 2024
@lrstewart lrstewart closed this Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant