-
-
Notifications
You must be signed in to change notification settings - Fork 452
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
Upgrade lengths of RichString to size_t
& many related changes.
#1592
base: main
Are you sure you want to change the base?
Changes from all commits
233c953
160c066
f83ab04
56654dd
9a395cb
9eb2343
473c847
7ff906c
bd27b7e
1715ad0
4ef38cb
fd54355
39c14cd
521e563
78184a1
e101339
9af21a7
4b32769
5d61426
22deed3
1b8f1ab
005c9b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -137,7 +137,7 @@ static void DiskIOMeter_display(ATTR_UNUSED const Object* cast, RichString* out) | |
|
||
int color = cached_utilisation_diff > 40.0 ? METER_VALUE_NOTICE : METER_VALUE; | ||
int len = xSnprintf(buffer, sizeof(buffer), "%.1f%%", cached_utilisation_diff); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar consideration re |
||
RichString_appendnAscii(out, CRT_colors[color], buffer, len); | ||
RichString_appendnAscii(out, CRT_colors[color], buffer, (unsigned int)len); | ||
|
||
RichString_appendAscii(out, CRT_colors[METER_TEXT], " read: "); | ||
RichString_appendAscii(out, CRT_colors[METER_VALUE_IOREAD], cached_read_diff_str); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ in the source distribution for its full text. | |
#include "MainPanel.h" | ||
|
||
#include <ctype.h> | ||
#include <stddef.h> | ||
#include <stdlib.h> | ||
#include <sys/types.h> | ||
|
||
|
@@ -81,7 +82,7 @@ static HandlerResult MainPanel_eventHandler(Panel* super, int ch) { | |
|
||
if (EVENT_IS_HEADER_CLICK(ch)) { | ||
int x = EVENT_HEADER_CLICK_GET_X(ch); | ||
int hx = super->scrollH + x + 1; | ||
size_t hx = super->scrollH + (unsigned int)x + 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the second cast also be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @BenBE Let me explain a bit:
When I made this patch, I made a personal rule that, if a zero extension would suffice, I would avoid sign extension. This also applies to the noisy-as-you-called-it changes about me casting the |
||
RowField field = RowField_keyAt(settings, hx); | ||
if (ss->treeView && ss->treeViewAlwaysByPID) { | ||
ss->treeView = false; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,46 +96,44 @@ static void BarMeterMode_draw(Meter* this, int x, int y, int w) { | |
// The text in the bar is right aligned; | ||
// Pad with maximal spaces and then calculate needed starting position offset | ||
RichString_begin(bar); | ||
RichString_appendChr(&bar, 0, ' ', w); | ||
RichString_appendChr(&bar, 0, ' ', (unsigned int)w); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks overall kinda noisy. The source code shouldn't be a casting show … ;-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the problem of policy and whether the compiler would complain about implicit casts to unsigned. Given there is a conditional If htop is compiled with gcc or clang's |
||
RichString_appendWide(&bar, 0, this->txtBuffer); | ||
|
||
int startPos = RichString_sizeVal(bar) - w; | ||
if (startPos > w) { | ||
size_t startPos = RichString_sizeVal(bar) - (unsigned int)w; | ||
if (startPos > (unsigned int)w) { | ||
// Text is too large for bar | ||
// Truncate meter text at a space character | ||
for (int pos = 2 * w; pos > w; pos--) { | ||
for (unsigned int pos = 2 * (unsigned int)w; pos > (unsigned int)w; pos--) { | ||
if (RichString_getCharVal(bar, pos) == ' ') { | ||
while (pos > w && RichString_getCharVal(bar, pos - 1) == ' ') | ||
while (pos > (unsigned int)w && RichString_getCharVal(bar, pos - 1) == ' ') | ||
pos--; | ||
startPos = pos - w; | ||
startPos = pos - (unsigned int)w; | ||
break; | ||
} | ||
} | ||
|
||
// If still too large, print the start not the end | ||
startPos = MINIMUM(startPos, w); | ||
startPos = MINIMUM(startPos, (unsigned int)w); | ||
} | ||
|
||
assert(startPos >= 0); | ||
assert(startPos <= w); | ||
assert(startPos + w <= RichString_sizeVal(bar)); | ||
assert(startPos <= (unsigned int)w); | ||
assert(startPos + (unsigned int)w <= RichString_sizeVal(bar)); | ||
|
||
int blockSizes[10]; | ||
unsigned int blockSizes[10]; | ||
|
||
// First draw in the bar[] buffer... | ||
int offset = 0; | ||
unsigned int offset = 0; | ||
for (uint8_t i = 0; i < this->curItems; i++) { | ||
double value = this->values[i]; | ||
if (isPositive(value) && this->total > 0.0) { | ||
value = MINIMUM(value, this->total); | ||
blockSizes[i] = ceil((value / this->total) * w); | ||
blockSizes[i] = (unsigned int)(int)ceil((value / this->total) * w); | ||
blockSizes[i] = MINIMUM(MINIMUM(INT_MAX - (unsigned int)x, (unsigned int)w) - offset, blockSizes[i]); | ||
} else { | ||
blockSizes[i] = 0; | ||
} | ||
int nextOffset = offset + blockSizes[i]; | ||
// (Control against invalid values) | ||
nextOffset = CLAMP(nextOffset, 0, w); | ||
for (int j = offset; j < nextOffset; j++) | ||
unsigned int nextOffset = offset + blockSizes[i]; | ||
for (unsigned int j = offset; j < nextOffset; j++) | ||
if (RichString_getCharVal(bar, startPos + j) == ' ') { | ||
if (CRT_colorScheme == COLORSCHEME_MONOCHROME) { | ||
assert(i < strlen(BarMeterMode_characters)); | ||
|
@@ -152,13 +150,12 @@ static void BarMeterMode_draw(Meter* this, int x, int y, int w) { | |
for (uint8_t i = 0; i < this->curItems; i++) { | ||
int attr = this->curAttributes ? this->curAttributes[i] : Meter_attributes(this)[i]; | ||
RichString_setAttrn(&bar, CRT_colors[attr], startPos + offset, blockSizes[i]); | ||
RichString_printoffnVal(bar, y, x + offset, startPos + offset, MINIMUM(blockSizes[i], w - offset)); | ||
RichString_printoffnVal(bar, y, x + (int)offset, startPos + offset, (int)blockSizes[i]); | ||
offset += blockSizes[i]; | ||
offset = CLAMP(offset, 0, w); | ||
} | ||
if (offset < w) { | ||
RichString_setAttrn(&bar, CRT_colors[BAR_SHADOW], startPos + offset, w - offset); | ||
RichString_printoffnVal(bar, y, x + offset, startPos + offset, w - offset); | ||
if (offset < (unsigned int)w) { | ||
RichString_setAttrn(&bar, CRT_colors[BAR_SHADOW], startPos + offset, (unsigned int)w - offset); | ||
RichString_printoffnVal(bar, y, x + (int)offset, startPos + offset, w - (int)offset); | ||
} | ||
|
||
RichString_delete(&bar); | ||
|
@@ -319,26 +316,28 @@ static void LEDMeterMode_draw(Meter* this, int x, int y, int w) { | |
attrset(CRT_colors[LED_COLOR]); | ||
const char* caption = Meter_getCaption(this); | ||
mvaddstr(yText, x, caption); | ||
int xx = x + strlen(caption); | ||
int len = RichString_sizeVal(out); | ||
for (int i = 0; i < len; i++) { | ||
int c = RichString_getCharVal(out, i); | ||
if (c >= '0' && c <= '9') { | ||
if (xx - x + 4 > w) | ||
break; | ||
|
||
LEDMeterMode_drawDigit(xx, y, c - '0'); | ||
xx += 4; | ||
} else { | ||
if (xx - x + 1 > w) | ||
break; | ||
if (strlen(caption) <= INT_MAX - (unsigned int)x) { | ||
int xx = x + (int)strlen(caption); | ||
size_t len = RichString_sizeVal(out); | ||
for (size_t i = 0; i < len; i++) { | ||
int c = RichString_getCharVal(out, i); | ||
if (c >= '0' && c <= '9') { | ||
if (xx - x + 4 > w) | ||
break; | ||
|
||
LEDMeterMode_drawDigit(xx, y, c - '0'); | ||
xx += 4; | ||
} else { | ||
if (xx - x + 1 > w) | ||
break; | ||
#ifdef HAVE_LIBNCURSESW | ||
const cchar_t wc = { .chars = { c, '\0' }, .attr = 0 }; /* use LED_COLOR from attrset() */ | ||
mvadd_wch(yText, xx, &wc); | ||
const cchar_t wc = { .chars = { c, '\0' }, .attr = 0 }; /* use LED_COLOR from attrset() */ | ||
mvadd_wch(yText, xx, &wc); | ||
#else | ||
mvaddch(yText, xx, c); | ||
mvaddch(yText, xx, c); | ||
#endif | ||
xx += 1; | ||
xx += 1; | ||
} | ||
} | ||
} | ||
attrset(CRT_colors[RESET_COLOR]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance to reduce noise by having
size_t len
and thus avoiding all the subsequent casts? We knowxSnprintf
can't be negative, which we already check for.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach just moves the casts from the
RichString
append functions toxSnprintf
. Unless we alter the API ofxSnprintf
, I don't think it reduces any noise overall.How about this alternate solution: Create a
RichString_appendAsciiFormat()
function that internally combinesxSnprintf
andRichString_appendnAscii()
?The prototype could look something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: I've added the commit aa74b30.