Remove all stack allocations #235
Conversation
| @@ -51,10 +50,12 @@ static void h3Line_assertions(H3Index start, H3Index end) { | |||
| "index is a neighbor of the previous index"); | |||
| if (i > 1) { | |||
| t_assert( | |||
| !H3_EXPORT(h3IndexesAreNeighbors)(line[i], line[i - 3]), | |||
| !H3_EXPORT(h3IndexesAreNeighbors)(line[i], line[i - 2]), | |||
There was a problem hiding this comment.
Is this a bugfix in the test? If so, how did it work before?
There was a problem hiding this comment.
That must be my bug - I'm guessing that the invalid array access (would be line[-1] on the first access) wasn't throwing an error with stack allocation, and since the assertion is a negative one, it passes on !h3IndexesAreNeighbors(h3Index, trash) - it would only fail if the trash access gave it an H3 index that was a neighbor.
Not sure how I made the mistake in the first place, but good catch!
| @@ -64,7 +65,7 @@ static void h3Line_invalid_assertions(H3Index start, H3Index end) { | |||
| int sz = H3_EXPORT(h3LineSize)(start, end); | |||
| t_assert(sz < 0, "line size marked as invalid"); | |||
|
|
|||
| H3Index* line = {0}; | |||
| H3Index *line = {0}; | |||
There was a problem hiding this comment.
I understand it's due to the linter, but I've never understood the C style to put the star next to the variable name. The type of the variable is a pointer to an H3Index, and you reference the variable with just the text part of the name, so the star seems to be more reasonably associated with that type.
There was a problem hiding this comment.
I think the reasoning for this linter rule is due to how C handles H3Index *a, b; which creates one pointer-to-H3Index and one H3Index.
| @@ -51,10 +50,12 @@ static void h3Line_assertions(H3Index start, H3Index end) { | |||
| "index is a neighbor of the previous index"); | |||
| if (i > 1) { | |||
| t_assert( | |||
| !H3_EXPORT(h3IndexesAreNeighbors)(line[i], line[i - 3]), | |||
| !H3_EXPORT(h3IndexesAreNeighbors)(line[i], line[i - 2]), | |||
There was a problem hiding this comment.
That must be my bug - I'm guessing that the invalid array access (would be line[-1] on the first access) wasn't throwing an error with stack allocation, and since the assertion is a negative one, it passes on !h3IndexesAreNeighbors(h3Index, trash) - it would only fail if the trash access gave it an H3 index that was a neighbor.
Not sure how I made the mistake in the first place, but good catch!
Remove all stack allocations
Fixes #233