Skip to content

Conversation

@jodavies
Copy link
Collaborator

@jodavies jodavies commented Oct 14, 2025

Issue #251 crashes because "fill" overtakes "s" and produces malformed tokens, leading to an infinite loop. Avoid this possibility by writing the output tokens into a scratch buffer, and copy the result back at the end.

Additionally, re-allocate the buffers if fill is "close to" the end of the scratch buffer. Here "close to" is a guess at how much space might be required per iteration.


Here is a proposed fix of #251. The "magic" +20 is no longer required, but instead there is a magic "how much space is required at the end of the token buffer for each iteration of the while loop?" At least in the future if someone reports that fill goes out of bounds of tmptokens it is clear what is going on.

Some TEMPTY space is still required at the start of AC.tokens before the call of simp3btoken: s and t pointers end up in this space even if initially they are moved beyond it.

I don't measure any performance penalty due to the allocations (they are not all that large) but in principle one could introduce a permanent AC.tmptokens to serve as scratch space without the allocations.

Any comments?

@coveralls
Copy link

coveralls commented Oct 15, 2025

Coverage Status

coverage: 56.902% (+0.008%) from 56.894%
when pulling cd7312a on jodavies:issue-251
into afebe66 on form-dev:master.

Issue form-dev#251 crashes because "fill" overtakes "s" and produces malformed tokens,
leading to an infinite loop. Avoid this possibility by writing the output
tokens into a scratch buffer, and copy the result back at the end.

Additionally, re-allocate the buffers if fill is "close to" the end of the
scratch buffer. Here "close to" is a guess at how much space might be required
per iteration.
@jodavies
Copy link
Collaborator Author

After a bit more benchmarking I still don't see any impact from the additional memory allocation. I think this patch is good to go. In cases where the +20 threshold for reallocation is not sufficient, the old code would crash also, and this fixes the problematic cases that we know about.

@jodavies jodavies merged commit 37dc3b6 into form-dev:master Oct 28, 2025
84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants