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

Scintilla 5.1.5 aftermath #3098

Merged
merged 1 commit into from Jan 11, 2022
Merged

Scintilla 5.1.5 aftermath #3098

merged 1 commit into from Jan 11, 2022

Conversation

kugel-
Copy link
Member

@kugel- kugel- commented Jan 11, 2022

This commit fixes a few problems introduced by the last Scintilla update.
That update caused some headache around the incompatible changes to
SCI_GETTEXT, SCI_GETSELTEXT, and SCI_GETCURLINE.

  • An explicit NUL termination was added to sci_get_text(). This is both
    superflous and wrong (it writes behind the allocated buffer) as SCI_GETTEXT
    already does NUL termination.

  • In sci_get_contents(), sci_get_string() cannot be used. That would call
    SCI_GETTEXT with length == 0 which is not the desired outcome. Instead,
    basically revert to the old implementation but account for the API change.

  • The callers of sci_get_selected_text_length() must be adapted, this
    was missing yet. sci_get_selected_text_length() return value does not
    include the NUL termination anymore.

Resolves #3095

Fixes: d7c985e ("Adapt to SCI_GETTEXT changes")

This commit fixes a few problems introduced by the last Scintilla update.
That update caused some headache around the incompatible changes to
`SCI_GETTEXT`, `SCI_GETSELTEXT`, and `SCI_GETCURLINE`.

- An explicit NUL termination was added to `sci_get_text()`. This is both
  superflous and wrong (it writes behind the allocated buffer) as SCI_GETTEXT
  already does NUL termination.

- In `sci_get_contents()`, sci_get_string() cannot be used. That would call
  SCI_GETTEXT with length == 0 which is not the desired outcome. Instead,
  basically revert to the old implementation but account for the API change.

- The callers of sci_get_selected_text_length() must be adapted, this
  was missing yet. sci_get_selected_text_length() return value does not
  include the NUL termination anymore.

Resolves geany#3095

Fixes: d7c985e ("Adapt to SCI_GETTEXT changes")
@techee
Copy link
Member

techee commented Jan 11, 2022

Just went through the patch and looks good to me. This should be basically all needed for Geany itself.

@kugel-
Copy link
Member Author

kugel- commented Jan 11, 2022

Thanks. I'll merge quickly as it resolves pretty severe issue.

@kugel- kugel- merged commit 142b8ff into geany:master Jan 11, 2022
@techee
Copy link
Member

techee commented Jan 11, 2022

Thanks. I'll merge quickly as it resolves pretty severe issue.

Pity. What a pleasant change to see that Geany is capable of crashing ;-).

@b4n b4n added this to the 1.39/2.0 milestone Feb 20, 2023
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.

Crash after update to Scintilla 5.1.5
3 participants