From d94e0eb8ede75a72af24df1dc76efd395b321bd9 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 4 Jun 2025 04:48:47 +0800 Subject: [PATCH 1/7] Remove an unnecessary cast in RowField_keyAt() No code changes. --- Row.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Row.c b/Row.c index 5fae97654..6a5a842f6 100644 --- a/Row.c +++ b/Row.c @@ -177,7 +177,7 @@ const char* RowField_alignedTitle(const Settings* settings, RowField field) { } RowField RowField_keyAt(const Settings* settings, int at) { - const RowField* fields = (const RowField*) settings->ss->fields; + const RowField* fields = settings->ss->fields; RowField field; int x = 0; for (int i = 0; (field = fields[i]); i++) { From 5fe6cfa9fb87ac54b74699c1e2ff732687d5770b Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 4 Jun 2025 04:48:50 +0800 Subject: [PATCH 2/7] Adjust some Meter.c conditionals for readability No code changes. --- Meter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Meter.c b/Meter.c index 59031d54c..b9bf25300 100644 --- a/Meter.c +++ b/Meter.c @@ -54,7 +54,7 @@ static void TextMeterMode_draw(Meter* this, int x, int y, int w) { assert(w <= INT_MAX - x); const char* caption = Meter_getCaption(this); - if (w >= 1) { + if (w > 0) { attrset(CRT_colors[METER_TEXT]); mvaddnstr(y, x, caption, w); } @@ -336,7 +336,7 @@ static void LEDMeterMode_draw(Meter* this, int x, int y, int w) { attrset(CRT_colors[LED_COLOR]); const char* caption = Meter_getCaption(this); - if (w >= 1) { + if (w > 0) { mvaddnstr(yText, x, caption, w); } From 7f5f1b566e7261f4bf00de55d6a7fb553db3a031 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 4 Jun 2025 04:55:54 +0800 Subject: [PATCH 3/7] Add a constant 'bracketWidth' in Action_setScreenTab() No changes in compiled code. --- Action.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Action.c b/Action.c index be88c0dd5..d3a800ceb 100644 --- a/Action.c +++ b/Action.c @@ -402,6 +402,7 @@ static Htop_Reaction actionPrevScreen(State* st) { Htop_Reaction Action_setScreenTab(State* st, int x) { Settings* settings = st->host->settings; + const int bracketWidth = (int)strlen("[]"); int s = SCREEN_TAB_MARGIN_LEFT; for (unsigned int i = 0; i < settings->nScreens; i++) { if (x < s) { @@ -409,12 +410,12 @@ Htop_Reaction Action_setScreenTab(State* st, int x) { } const char* tab = settings->screens[i]->heading; int len = strlen(tab); - if (x < s + len + 2) { + if (x < s + len + bracketWidth) { settings->ssIndex = i; setActiveScreen(settings, st, i); return HTOP_UPDATE_PANELHDR | HTOP_REFRESH | HTOP_REDRAW_BAR; } - s += len + 2 + SCREEN_TAB_COLUMN_GAP; + s += len + bracketWidth + SCREEN_TAB_COLUMN_GAP; } return 0; } From bb5696f2cddac609a537c77d3355a064dd832439 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 4 Jun 2025 04:56:56 +0800 Subject: [PATCH 4/7] Add assert and explicit cast in AffinityPanel name length This avoids a '-Wshorten-64-to-32' warning produced by Clang. Signed-off-by: Kang-Che Sung --- AffinityPanel.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/AffinityPanel.c b/AffinityPanel.c index ea1d85596..f007ae820 100644 --- a/AffinityPanel.c +++ b/AffinityPanel.c @@ -10,6 +10,7 @@ in the source distribution for its full text. #include "AffinityPanel.h" #include +#include // IWYU pragma: keep #include #include #include @@ -321,7 +322,9 @@ static MaskItem* AffinityPanel_addObject(AffinityPanel* this, hwloc_obj_t obj, u } /* "[x] " + "|- " * depth + ("- ")?(if root node) + name */ - unsigned width = 4 + 3 * depth + (2 * !depth) + strlen(buf); + unsigned int indent_width = 4 + 3 * depth + (2 * !depth); + assert(sizeof(buf) <= INT_MAX - indent_width); + unsigned int width = indent_width + (unsigned int)strlen(buf); if (width > this->width) { this->width = width; } From b75e833552730125f0663f77aedbdb6950954585 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 4 Jun 2025 04:57:05 +0800 Subject: [PATCH 5/7] Improve ScreenManager_drawScreenTabs() logic on screen width 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 --- ScreenManager.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ScreenManager.c b/ScreenManager.c index e325d8397..02a76b959 100644 --- a/ScreenManager.c +++ b/ScreenManager.c @@ -161,6 +161,9 @@ static void checkRecalculation(ScreenManager* this, double* oldTime, int* sortTi } static inline bool drawTab(const int* y, int* x, int l, const char* name, bool cur) { + assert(*x >= 0); + assert(*x < l); + attrset(CRT_colors[cur ? SCREENS_CUR_BORDER : SCREENS_OTH_BORDER]); mvaddch(*y, *x, '['); (*x)++; @@ -190,9 +193,12 @@ static void ScreenManager_drawScreenTabs(ScreenManager* this) { int y = panel->y - 1; int x = SCREEN_TAB_MARGIN_LEFT; + if (x >= l) + goto end; + if (this->name) { drawTab(&y, &x, l, this->name, true); - return; + goto end; } for (int s = 0; screens[s]; s++) { @@ -201,6 +207,8 @@ static void ScreenManager_drawScreenTabs(ScreenManager* this) { break; } } + +end: attrset(CRT_colors[RESET_COLOR]); } From a97a157172065c536c0725d6776aab8eeed5dfb9 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 4 Jun 2025 04:57:09 +0800 Subject: [PATCH 6/7] Add strnlen() function and configure check Add configure check for strnlen() function and provide our own implementation for systems that doesn't support it. Co-Authored-By: Benny Baumann Signed-off-by: Kang-Che Sung --- XUtils.c | 11 +++++++++++ XUtils.h | 4 ++++ configure.ac | 1 + 3 files changed, 16 insertions(+) diff --git a/XUtils.c b/XUtils.c index 5b71e7817..db5abef20 100644 --- a/XUtils.c +++ b/XUtils.c @@ -224,6 +224,17 @@ size_t String_safeStrncpy(char* restrict dest, const char* restrict src, size_t return i; } +#ifndef HAVE_STRNLEN +size_t strnlen(const char* str, size_t maxLen) { + for (size_t len = 0; len < maxLen; len++) { + if (!str[len]) { + return len; + } + } + return maxLen; +} +#endif + int xAsprintf(char** strp, const char* fmt, ...) { va_list vl; va_start(vl, fmt); diff --git a/XUtils.h b/XUtils.h index c9936f7df..32e9446df 100644 --- a/XUtils.h +++ b/XUtils.h @@ -102,6 +102,10 @@ static inline char* String_strchrnul(const char* s, int c) { ATTR_NONNULL ATTR_ACCESS3_W(1, 3) ATTR_ACCESS3_R(2, 3) size_t String_safeStrncpy(char* restrict dest, const char* restrict src, size_t size); +#ifndef HAVE_STRNLEN +size_t strnlen(const char* str, size_t maxLen); +#endif + ATTR_FORMAT(printf, 2, 3) ATTR_NONNULL_N(1, 2) int xAsprintf(char** strp, const char* fmt, ...); diff --git a/configure.ac b/configure.ac index 40bf7becb..391299e6a 100644 --- a/configure.ac +++ b/configure.ac @@ -372,6 +372,7 @@ AC_CHECK_FUNCS([ \ readlinkat \ sched_getscheduler \ sched_setscheduler \ + strnlen \ ]) # strchrnul is available in macOS since 15.4, but the user may specify an older From e9e80017a180f9f69a3a0805fb856951943951fa Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 4 Jun 2025 07:22:20 +0800 Subject: [PATCH 7/7] Use strnlen() for some string width calculations 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 #1673) as a result. Signed-off-by: Kang-Che Sung --- Action.c | 22 +++++++++++++++------- Meter.c | 14 +++++++------- Row.c | 8 ++++---- ScreenManager.c | 7 +++---- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/Action.c b/Action.c index d3a800ceb..1d3bccc51 100644 --- a/Action.c +++ b/Action.c @@ -403,19 +403,27 @@ static Htop_Reaction actionPrevScreen(State* st) { Htop_Reaction Action_setScreenTab(State* st, int x) { Settings* settings = st->host->settings; const int bracketWidth = (int)strlen("[]"); - int s = SCREEN_TAB_MARGIN_LEFT; + + if (x < SCREEN_TAB_MARGIN_LEFT) { + return 0; + } + + int rem = x - SCREEN_TAB_MARGIN_LEFT; for (unsigned int i = 0; i < settings->nScreens; i++) { - if (x < s) { - return 0; - } const char* tab = settings->screens[i]->heading; - int len = strlen(tab); - if (x < s + len + bracketWidth) { + int width = rem >= bracketWidth ? (int)strnlen(tab, rem - bracketWidth + 1) : 0; + if (width >= rem - bracketWidth + 1) { settings->ssIndex = i; setActiveScreen(settings, st, i); return HTOP_UPDATE_PANELHDR | HTOP_REFRESH | HTOP_REDRAW_BAR; } - s += len + bracketWidth + SCREEN_TAB_COLUMN_GAP; + + rem -= bracketWidth + width; + if (rem < SCREEN_TAB_COLUMN_GAP) { + return 0; + } + + rem -= SCREEN_TAB_COLUMN_GAP; } return 0; } diff --git a/Meter.c b/Meter.c index b9bf25300..7a9e5ff46 100644 --- a/Meter.c +++ b/Meter.c @@ -60,12 +60,12 @@ static void TextMeterMode_draw(Meter* this, int x, int y, int w) { } attrset(CRT_colors[RESET_COLOR]); - int captionLen = strlen(caption); - w -= captionLen; - if (w < 1) { + int captionWidth = w > 0 ? (int)strnlen(caption, w) : 0; + if (w <= captionWidth) { return; } - x += captionLen; + w -= captionWidth; + x += captionWidth; RichString_begin(out); Meter_displayBuffer(this, &out); @@ -340,11 +340,11 @@ static void LEDMeterMode_draw(Meter* this, int x, int y, int w) { mvaddnstr(yText, x, caption, w); } - int captionLen = strlen(caption); - if (w <= captionLen) { + int captionWidth = w > 0 ? (int)strnlen(caption, w) : 0; + if (w <= captionWidth) { goto end; } - int xx = x + captionLen; + int xx = x + captionWidth; #ifdef HAVE_LIBNCURSESW if (CRT_utf8) diff --git a/Row.c b/Row.c index 6a5a842f6..13e79e648 100644 --- a/Row.c +++ b/Row.c @@ -179,13 +179,13 @@ const char* RowField_alignedTitle(const Settings* settings, RowField field) { RowField RowField_keyAt(const Settings* settings, int at) { const RowField* fields = settings->ss->fields; RowField field; - int x = 0; + int rem = at; for (int i = 0; (field = fields[i]); i++) { - int len = strlen(RowField_alignedTitle(settings, field)); - if (at >= x && at <= x + len) { + int len = rem > 0 ? (int)strnlen(RowField_alignedTitle(settings, field), rem) : 0; + if (rem <= len) { return field; } - x += len; + rem -= len; } return COMM; } diff --git a/ScreenManager.c b/ScreenManager.c index 02a76b959..86e0fc3dd 100644 --- a/ScreenManager.c +++ b/ScreenManager.c @@ -169,11 +169,10 @@ static inline bool drawTab(const int* y, int* x, int l, const char* name, bool c (*x)++; if (*x >= l) return false; - int nameLen = strlen(name); - int n = MINIMUM(l - *x, nameLen); + int nameWidth = (int)strnlen(name, l - *x); attrset(CRT_colors[cur ? SCREENS_CUR_TEXT : SCREENS_OTH_TEXT]); - mvaddnstr(*y, *x, name, n); - *x += n; + mvaddnstr(*y, *x, name, nameWidth); + *x += nameWidth; if (*x >= l) return false; attrset(CRT_colors[cur ? SCREENS_CUR_BORDER : SCREENS_OTH_BORDER]);