Introduce strnlen() and use it for some string width calculations#1707
Merged
BenBE merged 7 commits intohtop-dev:mainfrom Jun 4, 2025
Merged
Introduce strnlen() and use it for some string width calculations#1707BenBE merged 7 commits intohtop-dev:mainfrom
BenBE merged 7 commits intohtop-dev:mainfrom
Conversation
BenBE
reviewed
May 22, 2025
BenBE
added
code quality ♻️
Explorer09
force-pushed
the
string-width
branch
2 times, most recently
from
0348583 to
b73e050
Compare
BenBE
reviewed
May 28, 2025
Explorer09
force-pushed
the
string-width
branch
2 times, most recently
from
43bfd83 to
d70cffe
Compare
Member
|
When clicking the screen tabs there's a small off-by-one present:
A similar issue exists when trying to select a column header:
|
Contributor
Author
Can't reproduce in my terminal emulator. They work perfectly on my side. Both on macOS Terminal and xfce4-terminal (on Devuan GNU/Linux). |
No code changes.
No code changes.
No changes in compiled code.
This avoids a '-Wshorten-64-to-32' warning produced by Clang. Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Make the code more robust with very small width of the terminal screen. Also add assertions on x positions in the drawTab() subroutine. Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Add configure check for strnlen() function and provide our own implementation for systems that doesn't support it. Co-Authored-By: Benny Baumann <BenBE@geshi.org> Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Specifically in these functions: Action_setScreenTab(), TextMeterMode_draw(), LEDMeterMode_draw() RowField_keyAt() and drawTab() The strnlen() function does not calculate true display widths of the Unicode strings, but at least it works with ASCII strings. The function that can calculate display widths of Unicode strings is yet to be implemented. The goal of this commit is to prevent potential arithmetic overflows when calculating string widths and to allow a safe downcast from 'size_t' to 'int' type. This fixes some of the '-Wshorten-64-to-32' warnings produced by Clang (see issue htop-dev#1673) as a result. Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
Member
|
Merging for now, as this happens for me also with the previous version without this PR applied. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
strnlenfunction does not calculate true display widths of the Unicode strings, but at least it works with ASCII strings. The function that can calculate display widths of Unicode strings is not implemented in htop yet (I have #1642, but the implementation of that is incomplete).The goal of this commit is to prevent potential arithmetic overflows when calculating string widths and to allow a safe downcast from
size_ttointtype. This fixes some of the-Wshorten-64-to-32warnings produced by Clang (see #1673) as a result.