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

Fixed comparison of pointer addition with NULL #549

Merged
merged 1 commit into from
May 16, 2024

Conversation

tsujan
Copy link
Member

@tsujan tsujan commented May 9, 2024

The comparison was added to fix a crash under Plasma-Wayland, when the terminal was split horizontally. Instead, a simple nullity check was enough.

The following compilation warning is also silenced:

qtermwidget/lib/TerminalCharacterDecoder.cpp:85:28: warning: comparing the result of pointer addition ‘(((const Konsole::Character*)characters) + ((sizetype)(((long unsigned int)i) * 16)))’ and NULL [-Waddress]
   85 |         if (characters + i == nullptr)

The comparison was added to fix a crash under Plasma-Wayland, when the terminal was split horizontally. Instead, a simple nullity check was enough.

The following compilation warning is also silenced:

```
qtermwidget/lib/TerminalCharacterDecoder.cpp:85:28: warning: comparing the result of pointer addition ‘(((const Konsole::Character*)characters) + ((sizetype)(((long unsigned int)i) * 16)))’ and NULL [-Waddress]
   85 |         if (characters + i == nullptr)
```
@luis-pereira
Copy link
Member

I don't have Plasma installed to check it. That said, it seems Ok to me.

@tsujan
Copy link
Member Author

tsujan commented May 16, 2024

I don't have Plasma installed to check it.

I tested it with kwin_wayland, and it prevented the crash that was reported at lxqt/qterminal#806.

@tsujan tsujan merged commit f971c36 into master May 16, 2024
2 checks passed
@tsujan tsujan deleted the removed_ponter_addition branch May 16, 2024 16:07
@luis-pereira
Copy link
Member

I tested it with kwin_wayland, and it prevented the crash that was reported at lxqt/qterminal#806

Good. Didn't found any issue when not running without kwin_wayland.

@tsujan
Copy link
Member Author

tsujan commented May 16, 2024

Didn't found any issue when not running without kwin_wayland.

Yes. For some reason unknown to us, it didn't happen in any Wayland compositor other than kwin_wayland — I'd checked with Weston, GNOME, LabWc and Wayfire.

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