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

Fix crash in #103 #104

Merged
merged 1 commit into from Oct 4, 2022
Merged

Fix crash in #103 #104

merged 1 commit into from Oct 4, 2022

Conversation

sgn
Copy link
Contributor

@sgn sgn commented Oct 3, 2022

The `u32_move` will try to read `input.lines[j].num_chars - c + 1` `u32`
octets from `input.lines[j].mbtext + input.lines[j].posmap[c]`. That
means, it needs access memory at address
`input.lines[j].mbtext + input.lines[j].posmap[c] + input.lines[j].num_chars - c`
while the max range is `input.lines[j].mbtext + input.lines[j].num_chars`,
which is out-of-bound because `input.lines[j].posmap[c] > c` obviously.

Fix ascii-boxes#103
@tsjensen
Copy link
Member

tsjensen commented Oct 4, 2022

Thank you for providing us with a fix! 👍

Before I merge, I'd like to understand the change a little better, if that's OK with you. In the call to u32_move(), you changed the number of characters moved. It is calculated as input.lines[j].num_chars - input.lines[j].posmap[c] + 1, where num_chars is in characters, but posmap[c] is in bytes. How does that result in a valid number? Or is my assumption about num_chars incorrect?

@sgn
Copy link
Contributor Author

sgn commented Oct 4, 2022

Your assumption about num_chars is correct. It's counted by characters. However, posmap[c] is in characters, too.
Quoting your code:

size_t *posmap; /* for each character in `text`, position of corresponding char in `mbtext`. Needed for box removal. */

@tsjensen
Copy link
Member

tsjensen commented Oct 4, 2022

Ah, right, I get it now. I literally wrote this in the previous millennium, so bear with me. 😊

posmap gives the position in mbtext for each visible character. All the invisible characters, like ANSI color codes, are ignored. The full posmap looks like this:

image

In this test case, we want to remove 3 spaces from the beginning of the string, which means that we must remove 55 characters, including lots of invisible ones. So, as you said from the start, we were moving too many characters.

@tsjensen tsjensen merged commit 61562b0 into ascii-boxes:master Oct 4, 2022
@tsjensen tsjensen linked an issue Oct 4, 2022 that may be closed by this pull request
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.

Tests failing on x86_64-unknown-linux-musl
2 participants