From 577416d1a946382ab9f0c523e5fae755f9d71f69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 5 Oct 2020 12:49:01 +0200 Subject: [PATCH 1/2] Assert allocating non-zero size memory Allocating zero size memory results in implementation-defined behavior: man:malloc(3) : If size is 0, then malloc() returns either NULL, or a unique pointer value that can later be successfully passed to free(). --- Meter.c | 2 +- XUtils.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Meter.c b/Meter.c index 84370362..fa1eacb4 100644 --- a/Meter.c +++ b/Meter.c @@ -38,7 +38,7 @@ Meter* Meter_new(struct ProcessList_* pl, int param, const MeterClass* type) { this->param = param; this->pl = pl; this->curItems = type->maxItems; - this->values = xCalloc(type->maxItems, sizeof(double)); + this->values = type->maxItems ? xCalloc(type->maxItems, sizeof(double)) : NULL; this->total = type->total; this->caption = xStrdup(type->caption); if (Meter_initFn(this)) diff --git a/XUtils.c b/XUtils.c index 1e77cd3b..3bdcba84 100644 --- a/XUtils.c +++ b/XUtils.c @@ -25,26 +25,26 @@ void fail() { } void* xMalloc(size_t size) { + assert(size > 0); void* data = malloc(size); - if (!data && size > 0) { + if (!data) { fail(); } return data; } void* xCalloc(size_t nmemb, size_t size) { + assert(nmemb > 0); + assert(size > 0); void* data = calloc(nmemb, size); - if (!data && nmemb > 0 && size > 0) { + if (!data) { fail(); } return data; } void* xRealloc(void* ptr, size_t size) { - if (!size) { - free(ptr); - return NULL; - } + assert(size > 0); void* data = realloc(ptr, size); // deepcode ignore MemoryLeakOnRealloc: this goes to fail() if (!data) { free(ptr); From 4c66eb6d4cbdddc658e5f0274d8130155c6013f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Sat, 3 Oct 2020 21:20:43 +0200 Subject: [PATCH 2/2] XUtils string related updates - allow count out-parameter of String_split() to be NULL - introduce xStrndup() - do not allow NULL pointers passed to String_eq() it is not used in any code - implement String_startsWith(), String_contains_i() and String_eq() as inline header functions - adjust several conversion issues --- Action.h | 2 ++ CRT.c | 2 +- FunctionBar.c | 2 ++ MainPanel.h | 2 ++ Meter.h | 2 ++ Settings.c | 11 +++---- UsersTable.c | 2 ++ XUtils.c | 70 +++++++++++++++++++--------------------- XUtils.h | 23 +++++++++---- htop.c | 2 +- linux/Battery.c | 4 +-- linux/LinuxProcessList.c | 18 +++++------ linux/Platform.c | 2 +- 13 files changed, 77 insertions(+), 65 deletions(-) diff --git a/Action.h b/Action.h index e63b6810..68aa2e2e 100644 --- a/Action.h +++ b/Action.h @@ -7,6 +7,8 @@ Released under the GNU GPLv2, see the COPYING file in the source distribution for its full text. */ +#include "config.h" // IWYU pragma: keep + #include #include diff --git a/CRT.c b/CRT.c index d3afd0ec..a2efb6a1 100644 --- a/CRT.c +++ b/CRT.c @@ -655,7 +655,7 @@ void CRT_init(int delay, int colorScheme, bool allowUnicode) { setlocale(LC_CTYPE, ""); #ifdef HAVE_LIBNCURSESW - if (allowUnicode && strcmp(nl_langinfo(CODESET), "UTF-8") == 0) + if (allowUnicode && String_eq(nl_langinfo(CODESET), "UTF-8")) CRT_utf8 = true; else CRT_utf8 = false; diff --git a/FunctionBar.c b/FunctionBar.c index 1c4f395b..9b454ac8 100644 --- a/FunctionBar.c +++ b/FunctionBar.c @@ -5,6 +5,8 @@ Released under the GNU GPLv2, see the COPYING file in the source distribution for its full text. */ +#include "config.h" // IWYU pragma: keep + #include "FunctionBar.h" #include diff --git a/MainPanel.h b/MainPanel.h index 7f7520d6..3c555294 100644 --- a/MainPanel.h +++ b/MainPanel.h @@ -8,6 +8,8 @@ Released under the GNU GPLv2, see the COPYING file in the source distribution for its full text. */ +#include "config.h" // IWYU pragma: keep + #include #include diff --git a/Meter.h b/Meter.h index 84d157bb..ab8ee6e4 100644 --- a/Meter.h +++ b/Meter.h @@ -7,6 +7,8 @@ Released under the GNU GPLv2, see the COPYING file in the source distribution for its full text. */ +#include "config.h" // IWYU pragma: keep + #include #include diff --git a/Settings.c b/Settings.c index ac8321a0..879d14fb 100644 --- a/Settings.c +++ b/Settings.c @@ -31,16 +31,14 @@ void Settings_delete(Settings* this) { static void Settings_readMeters(Settings* this, char* line, int column) { char* trim = String_trim(line); - int nIds; - char** ids = String_split(trim, ' ', &nIds); + char** ids = String_split(trim, ' ', NULL); free(trim); this->columns[column].names = ids; } static void Settings_readMeterModes(Settings* this, char* line, int column) { char* trim = String_trim(line); - int nIds; - char** ids = String_split(trim, ' ', &nIds); + char** ids = String_split(trim, ' ', NULL); free(trim); int len = 0; for (int i = 0; ids[i]; i++) { @@ -94,8 +92,7 @@ static void Settings_defaultMeters(Settings* this, int initialCpuCount) { static void readFields(ProcessField* fields, int* flags, const char* line) { char* trim = String_trim(line); - int nIds; - char** ids = String_split(trim, ' ', &nIds); + char** ids = String_split(trim, ' ', NULL); free(trim); int i, j; *flags = 0; @@ -126,7 +123,7 @@ static bool Settings_read(Settings* this, const char* fileName, int initialCpuCo if (!line) { break; } - int nOptions; + size_t nOptions; char** option = String_split(line, '=', &nOptions); free (line); if (nOptions < 2) { diff --git a/UsersTable.c b/UsersTable.c index 89fdfc4d..83c6dbc3 100644 --- a/UsersTable.c +++ b/UsersTable.c @@ -5,6 +5,8 @@ Released under the GNU GPLv2, see the COPYING file in the source distribution for its full text. */ +#include "config.h" // IWYU pragma: keep + #include "UsersTable.h" #include diff --git a/XUtils.c b/XUtils.c index 3bdcba84..a444a7f8 100644 --- a/XUtils.c +++ b/XUtils.c @@ -54,11 +54,11 @@ void* xRealloc(void* ptr, size_t size) { } char* String_cat(const char* s1, const char* s2) { - int l1 = strlen(s1); - int l2 = strlen(s2); + const size_t l1 = strlen(s1); + const size_t l2 = strlen(s2); char* out = xMalloc(l1 + l2 + 1); memcpy(out, s1, l1); - memcpy(out+l1, s2, l2+1); + memcpy(out+l1, s2, l2); out[l1 + l2] = '\0'; return out; } @@ -67,39 +67,24 @@ char* String_trim(const char* in) { while (in[0] == ' ' || in[0] == '\t' || in[0] == '\n') { in++; } - int len = strlen(in); + + size_t len = strlen(in); while (len > 0 && (in[len-1] == ' ' || in[len-1] == '\t' || in[len-1] == '\n')) { len--; } - char* out = xMalloc(len+1); - strncpy(out, in, len); - out[len] = '\0'; - return out; + + return xStrndup(in, len); } -inline int String_eq(const char* s1, const char* s2) { - if (s1 == NULL || s2 == NULL) { - if (s1 == NULL && s2 == NULL) - return 1; - else - return 0; - } - return (strcmp(s1, s2) == 0); -} - -char** String_split(const char* s, char sep, int* n) { - *n = 0; - const int rate = 10; +char** String_split(const char* s, char sep, size_t* n) { + const unsigned int rate = 10; char** out = xCalloc(rate, sizeof(char*)); - int ctr = 0; - int blocks = rate; - char* where; + size_t ctr = 0; + unsigned int blocks = rate; + const char* where; while ((where = strchr(s, sep)) != NULL) { - int size = where - s; - char* token = xMalloc(size + 1); - strncpy(token, s, size); - token[size] = '\0'; - out[ctr] = token; + size_t size = (size_t)(where - s); + out[ctr] = xStrndup(s, size); ctr++; if (ctr == blocks) { blocks += rate; @@ -113,7 +98,10 @@ char** String_split(const char* s, char sep, int* n) { } out = xRealloc(out, sizeof(char*) * (ctr + 1)); out[ctr] = NULL; - *n = ctr; + + if (n) + *n = ctr; + return out; } @@ -121,28 +109,28 @@ void String_freeArray(char** s) { if (!s) { return; } - for (int i = 0; s[i] != NULL; i++) { + for (size_t i = 0; s[i] != NULL; i++) { free(s[i]); } free(s); } char* String_getToken(const char* line, const unsigned short int numMatch) { - const unsigned short int len = strlen(line); + const size_t len = strlen(line); char inWord = 0; unsigned short int count = 0; char match[50]; - unsigned short int foundCount = 0; + size_t foundCount = 0; - for (unsigned short int i = 0; i < len; i++) { + for (size_t i = 0; i < len; i++) { char lastState = inWord; inWord = line[i] == ' ' ? 0:1; if (lastState == 0 && inWord == 1) count++; - if(inWord == 1){ + if (inWord == 1){ if (count == numMatch && line[i] != ' ' && line[i] != '\0' && line[i] != '\n' && line[i] != (char)EOF) { match[foundCount] = line[i]; foundCount++; @@ -155,8 +143,8 @@ char* String_getToken(const char* line, const unsigned short int numMatch) { } char* String_readLine(FILE* fd) { - const int step = 1024; - int bufSize = step; + const unsigned int step = 1024; + unsigned int bufSize = step; char* buffer = xMalloc(step + 1); char* at = buffer; for (;;) { @@ -213,3 +201,11 @@ char* xStrdup(const char* str) { } return data; } + +char* xStrndup(const char* str, size_t len) { + char* data = strndup(str, len); + if (!data) { + fail(); + } + return data; +} diff --git a/XUtils.h b/XUtils.h index f30af6f3..591e98bb 100644 --- a/XUtils.h +++ b/XUtils.h @@ -9,6 +9,7 @@ in the source distribution for its full text. #include "config.h" // IWYU pragma: keep +#include #include #include // IWYU pragma: keep #include // IWYU pragma: keep @@ -24,25 +25,31 @@ void* xCalloc(size_t nmemb, size_t size); void* xRealloc(void* ptr, size_t size); -#define String_startsWith(s, match) (strncmp((s),(match),strlen(match)) == 0) -#define String_contains_i(s1, s2) (strcasestr(s1, s2) != NULL) - /* * String_startsWith gives better performance if strlen(match) can be computed * at compile time (e.g. when they are immutable string literals). :) */ +static inline bool String_startsWith(const char* s, const char* match) { + return strncmp(s, match, strlen(match)) == 0; +} + +static inline bool String_contains_i(const char* s1, const char* s2) { + return strcasestr(s1, s2) != NULL; +} + +static inline bool String_eq(const char* s1, const char* s2) { + return strcmp(s1, s2) == 0; +} char* String_cat(const char* s1, const char* s2); char* String_trim(const char* in); -int String_eq(const char* s1, const char* s2); - -char** String_split(const char* s, char sep, int* n); +char** String_split(const char* s, char sep, size_t* n); void String_freeArray(char** s); -char* String_getToken(const char* line, const unsigned short int numMatch); +char* String_getToken(const char* line, unsigned short int numMatch); char* String_readLine(FILE* fd); @@ -54,4 +61,6 @@ int xSnprintf(char *buf, int len, const char* fmt, ...); char* xStrdup(const char* str) ATTR_NONNULL; +char* xStrndup(const char* str, size_t len) ATTR_NONNULL; + #endif diff --git a/htop.c b/htop.c index 7d1c1b16..840f24c3 100644 --- a/htop.c +++ b/htop.c @@ -122,7 +122,7 @@ static CommandLineSettings parseArguments(int argc, char** argv) { exit(0); case 's': assert(optarg); /* please clang analyzer, cause optarg can be NULL in the 'u' case */ - if (strcmp(optarg, "help") == 0) { + if (String_eq(optarg, "help")) { for (int j = 1; j < Platform_numberOfFields; j++) { const char* name = Process_fields[j].name; if (name) printf ("%s\n", name); diff --git a/linux/Battery.c b/linux/Battery.c index f3e2ff24..956a7397 100644 --- a/linux/Battery.c +++ b/linux/Battery.c @@ -53,7 +53,7 @@ static unsigned long int parseBatInfo(const char *fileName, const unsigned short if (!dirEntry) break; char* entryName = dirEntry->d_name; - if (strncmp(entryName, "BAT", 3)) + if (String_startsWith(entryName, "BAT")) continue; batteries[nBatteries] = xStrdup(entryName); nBatteries++; @@ -128,7 +128,7 @@ static ACPresence procAcpiCheck(void) { char *isOnline = String_getToken(line, 2); free(line); - if (strcmp(isOnline, "on-line") == 0) { + if (String_eq(isOnline, "on-line")) { isOn = AC_PRESENT; } else { isOn = AC_ABSENT; diff --git a/linux/LinuxProcessList.c b/linux/LinuxProcessList.c index 1ed13826..4e3a804f 100644 --- a/linux/LinuxProcessList.c +++ b/linux/LinuxProcessList.c @@ -419,9 +419,9 @@ static void LinuxProcessList_readIoFile(LinuxProcess* process, const char* dirna while ((line = strsep(&buf, "\n")) != NULL) { switch (line[0]) { case 'r': - if (line[1] == 'c' && strncmp(line+2, "har: ", 5) == 0) + if (line[1] == 'c' && String_startsWith(line+2, "har: ")) process->io_rchar = strtoull(line+7, NULL, 10); - else if (strncmp(line+1, "ead_bytes: ", 11) == 0) { + else if (String_startsWith(line+1, "ead_bytes: ")) { process->io_read_bytes = strtoull(line+12, NULL, 10); process->io_rate_read_bps = ((double)(process->io_read_bytes - last_read))/(((double)(now - process->io_rate_read_time))/1000); @@ -429,9 +429,9 @@ static void LinuxProcessList_readIoFile(LinuxProcess* process, const char* dirna } break; case 'w': - if (line[1] == 'c' && strncmp(line+2, "har: ", 5) == 0) + if (line[1] == 'c' && String_startsWith(line+2, "har: ")) process->io_wchar = strtoull(line+7, NULL, 10); - else if (strncmp(line+1, "rite_bytes: ", 12) == 0) { + else if (String_startsWith(line+1, "rite_bytes: ")) { process->io_write_bytes = strtoull(line+13, NULL, 10); process->io_rate_write_bps = ((double)(process->io_write_bytes - last_write))/(((double)(now - process->io_rate_write_time))/1000); @@ -439,14 +439,14 @@ static void LinuxProcessList_readIoFile(LinuxProcess* process, const char* dirna } break; case 's': - if (line[4] == 'r' && strncmp(line+1, "yscr: ", 6) == 0) { + if (line[4] == 'r' && String_startsWith(line+1, "yscr: ")) { process->io_syscr = strtoull(line+7, NULL, 10); - } else if (strncmp(line+1, "yscw: ", 6) == 0) { + } else if (String_startsWith(line+1, "yscw: ")) { process->io_syscw = strtoull(line+7, NULL, 10); } break; case 'c': - if (strncmp(line+1, "ancelled_write_bytes: ", 22) == 0) { + if (String_startsWith(line+1, "ancelled_write_bytes: ")) { process->io_cancelled_write_bytes = strtoull(line+23, NULL, 10); } } @@ -598,7 +598,7 @@ static void LinuxProcessList_readOpenVZData(LinuxProcess* process, const char* d switch(field) { case 1: foundEnvID = true; - if(0 != strcmp(name_value_sep, process->ctid ? process->ctid : "")) { + if(!String_eq(name_value_sep, process->ctid ? process->ctid : "")) { free(process->ctid); process->ctid = xStrdup(name_value_sep); } @@ -762,7 +762,7 @@ static void LinuxProcessList_readSecattrData(LinuxProcess* process, const char* char *newline = strchr(buffer, '\n'); if (newline) *newline = '\0'; - if (process->secattr && 0 == strcmp(process->secattr, buffer)) + if (process->secattr && String_eq(process->secattr, buffer)) return; free(process->secattr); process->secattr = xStrdup(buffer); diff --git a/linux/Platform.c b/linux/Platform.c index 1bb476ab..afa2b7fd 100644 --- a/linux/Platform.c +++ b/linux/Platform.c @@ -376,7 +376,7 @@ void Platform_getNetworkIO(unsigned long int *bytesReceived, &packetsTransmittedParsed) != 5) continue; - if (0 == strcmp(interfaceName, "lo:")) + if (String_eq(interfaceName, "lo:")) continue; bytesReceivedSum += bytesReceivedParsed;