Skip to content

fix: fixes issue #2183.#2184

Closed
paxcut wants to merge 6 commits intoWerWolv:masterfrom
paxcut:EmptyLineHomeCrash
Closed

fix: fixes issue #2183.#2184
paxcut wants to merge 6 commits intoWerWolv:masterfrom
paxcut:EmptyLineHomeCrash

Conversation

@paxcut
Copy link
Copy Markdown
Collaborator

@paxcut paxcut commented Mar 26, 2025

Fixes issue #2183 Find details there but basically I forgot to check for empty lines before trying to find first non-space char.

Forgot to check for empty lines before trying to find first non-space char.
auto home=0;
while (isspace(line[home].mChar))
home++;
if (line.size() > 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't fix the case where the line is non-empty but only has spaces. Something like while(home < line.size() && line[home]..) is probably the right fix here. 🙂

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by not fixing that case? I tested that and it doesnt crash anymore. The way I understood the request to make the home key skip initial spaces is that the right answer should be the end of the string of spaces at the beginning of the line regardless of the existence of any characters following them. When the last space is found the cursor is placed after it and before the next line so that should work fine. The Home key can become equivalent to the end key in special cases like this just like the end key becomes equivalent to the home key in an empty line.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested if it crashes but I'd expect it to go out of bounds at the end of the line.
With a line of [' ', ' '] it'll increment home to 2 and then check line[2] which is should fail right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cant test the latest version because Im in the process of compiling 150 files and it can take a while on my slow computer. While it is true that it seems like it should crash, I tested a line of spaces on an older version and it didn't crash so i assume it won't either in this one, but until i can debug it I can't really say why or even if it still works.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you were right, but going out of bounds for a non-empty vector doesn't crash ImHex for some reason so i thought it was finding a new line or end of line char but there is nothing like that. I added the code you posted to the fix to commit in a bit when I finish what I'm currently doing. thanks for the help.

…s incomplete and didn't cover the case of a non-empty line made out of paces entirely. Although it looked like ot worked ok, under debugging it was apparent that it was reading a vector out of bounds which can cause memory corruption errors that cn be hard to track down. Implemented code suggested by the issue author and this seems to fix the problem completely. Thanks!
@paxcut
Copy link
Copy Markdown
Collaborator Author

paxcut commented May 1, 2025

The changes in this PR are superseded by the changes in PR 2193 that implements all the changes proposed here.

@paxcut paxcut closed this May 1, 2025
@paxcut paxcut deleted the EmptyLineHomeCrash branch July 15, 2025 14:33
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