From dfec58e5dced9fd794cc4a8e7a88a4d34f0cacda Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Fri, 8 Dec 2023 00:47:18 -0600 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Use=20strlcpy=20with=20buf?= =?UTF-8?q?fer=20size=20(#26513)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Marlin/src/HAL/LINUX/include/Arduino.h | 3 ++ Marlin/src/core/mstring.h | 35 ++++++++----------- Marlin/src/gcode/queue.h | 2 +- .../lcd/extui/anycubic_chiron/chiron_tft.cpp | 13 ++++--- .../src/lcd/extui/anycubic_vyper/dgus_tft.cpp | 9 ++--- Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp | 2 +- .../src/lcd/extui/mks_ui/draw_print_file.cpp | 10 +++--- Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp | 3 +- .../extui/mks_ui/tft_lvgl_configuration.cpp | 6 ++-- Marlin/src/lcd/extui/nextion/nextion_tft.cpp | 3 +- Marlin/src/lcd/lcdprint.cpp | 10 +++--- Marlin/src/lcd/menu/menu_item.h | 6 ++-- Marlin/src/module/settings.cpp | 2 +- Marlin/src/sd/cardreader.cpp | 8 ++--- 14 files changed, 51 insertions(+), 61 deletions(-) diff --git a/Marlin/src/HAL/LINUX/include/Arduino.h b/Marlin/src/HAL/LINUX/include/Arduino.h index f05aaed880..6e9c80ee07 100644 --- a/Marlin/src/HAL/LINUX/include/Arduino.h +++ b/Marlin/src/HAL/LINUX/include/Arduino.h @@ -28,6 +28,9 @@ #include +#define strlcpy(A, B, C) strncpy(A, B, (C) - 1) +#define strlcpy_P(A, B, C) strncpy_P(A, B, (C) - 1) + #define HIGH 0x01 #define LOW 0x00 diff --git a/Marlin/src/core/mstring.h b/Marlin/src/core/mstring.h index 9606fa22af..0ea53fef1b 100644 --- a/Marlin/src/core/mstring.h +++ b/Marlin/src/core/mstring.h @@ -48,7 +48,7 @@ #define DEFAULT_MSTRING_SIZE 20 #endif -//#define UNSAFE_MSTRING // Don't initialize the string and don't terminate strncpy +//#define UNSAFE_MSTRING // Don't initialize the string to "" or set a terminating nul //#define USE_SPRINTF // Use sprintf instead of snprintf //#define DJB2_HASH // 32-bit hash with Djb2 algorithm //#define MSTRING_DEBUG // Debug string operations to diagnose memory leaks @@ -98,13 +98,7 @@ public: void debug(FSTR_P const f) { #if ENABLED(MSTRING_DEBUG) - SERIAL_ECHO(FTOP(f)); - SERIAL_CHAR(':'); - SERIAL_ECHO(uintptr_t(str)); - SERIAL_CHAR(' '); - SERIAL_ECHO(length()); - SERIAL_CHAR(' '); - SERIAL_ECHOLN(str); + SERIAL_ECHOLN(f, ':', uintptr_t(str), ' ', length(), ' ', str); #endif } @@ -112,12 +106,12 @@ public: // Chainable String Setters MString& set() { str[0] = '\0'; debug(F("clear")); return *this; } - MString& set(char *s) { strncpy(str, s, SIZE); debug(F("string")); return *this; } + MString& set(char *s) { strlcpy(str, s, SIZE + 1); debug(F("string")); return *this; } MString& set(const char *s) { return set(const_cast(s)); } - MString& set_P(PGM_P const s) { strncpy_P(str, s, SIZE); debug(F("pstring")); return *this; } + MString& set_P(PGM_P const s) { strlcpy_P(str, s, SIZE + 1); debug(F("pstring")); return *this; } MString& set(FSTR_P const f) { return set_P(FTOP(f)); } MString& set(const bool &b) { return set(b ? F("true") : F("false")); } - MString& set(const char c) { str[0] = c; if (1 < SIZE) str[1] = '\0'; debug(F("char")); return *this; } + MString& set(const char c) { str[0] = c; str[1] = '\0'; debug(F("char")); return *this; } MString& set(const int8_t &i) { SNPRINTF_P(str, SIZE, PSTR("%d"), i); debug(F("int8_t")); return *this; } MString& set(const short &i) { SNPRINTF_P(str, SIZE, PSTR("%d"), i); debug(F("short")); return *this; } MString& set(const int &i) { SNPRINTF_P(str, SIZE, PSTR("%d"), i); debug(F("int")); return *this; } @@ -134,11 +128,11 @@ public: MString& set(const xyze_pos_t &v) { set(); return append(v); } template - MString& set(const MString &m) { strncpy(str, &m, SIZE); debug(F("MString")); return *this; } + MString& set(const MString &m) { strlcpy(str, &m, SIZE + 1); debug(F("MString")); return *this; } - MString& setn(char *s, int len) { int c = _MIN(len, SIZE); strncpy(str, s, c); str[c] = '\0'; debug(F("string")); return *this; } + MString& setn(char *s, int len) { int c = _MIN(len, SIZE); strlcpy(str, s, c + 1); debug(F("string")); return *this; } MString& setn(const char *s, int len) { return setn(const_cast(s), len); } - MString& setn_P(PGM_P const s, int len) { int c = _MIN(len, SIZE); strncpy_P(str, s, c); str[c] = '\0'; debug(F("pstring")); return *this; } + MString& setn_P(PGM_P const s, int len) { int c = _MIN(len, SIZE); strlcpy_P(str, s, c + 1); debug(F("pstring")); return *this; } MString& setn(FSTR_P const f, int len) { return setn_P(FTOP(f), len); } // set(repchr_t('-', 10)) @@ -159,9 +153,9 @@ public: // Chainable String appenders MString& append() { debug(F("nil")); return *this; } // for macros that might emit no output - MString& append(char *s) { int sz = length(); if (sz < SIZE) strncpy(str + sz, s, SIZE - sz); debug(F("string")); return *this; } + MString& append(char *s) { int sz = length(); if (sz < SIZE) strlcpy(str + sz, s, SIZE - sz + 1); debug(F("string")); return *this; } MString& append(const char *s) { return append(const_cast(s)); } - MString& append_P(PGM_P const s) { int sz = length(); if (sz < SIZE) strncpy_P(str + sz, s, SIZE - sz); debug(F("pstring")); return *this; } + MString& append_P(PGM_P const s) { int sz = length(); if (sz < SIZE) strlcpy_P(str + sz, s, SIZE - sz + 1); debug(F("pstring")); return *this; } MString& append(FSTR_P const f) { return append_P(FTOP(f)); } MString& append(const bool &b) { return append(b ? F("true") : F("false")); } MString& append(const char c) { int sz = length(); if (sz < SIZE) { str[sz] = c; if (sz < SIZE - 1) str[sz + 1] = '\0'; } return *this; } @@ -195,15 +189,15 @@ public: MString& append(const MString &m) { return append(&m); } // Append only if the given space is available - MString& appendn(char *s, int len) { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strncpy(str + sz, s, c); str[sz + c] = '\0'; } debug(F("string")); return *this; } + MString& appendn(char *s, int len) { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strlcpy(str + sz, s, c + 1); } debug(F("string")); return *this; } MString& appendn(const char *s, int len) { return appendn(const_cast(s), len); } - MString& appendn_P(PGM_P const s, int len) { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strncpy_P(str + sz, s, c); str[sz + c] = '\0'; } debug(F("pstring")); return *this; } + MString& appendn_P(PGM_P const s, int len) { int sz = length(), c = _MIN(len, SIZE - sz); if (c > 0) { strlcpy_P(str + sz, s, c + 1); } debug(F("pstring")); return *this; } MString& appendn(FSTR_P const f, int len) { return appendn_P(FTOP(f), len); } // append(repchr_t('-', 10)) MString& append(const repchr_t &s) { const int sz = length(), c = _MIN(s.count, SIZE - sz); - if (c > 0) { memset(str + sz, s.asc, c); safety(sz + c); } + if (c > 0) { memset(str + sz, s.asc, c); str[sz + c] = '\0'; } debug(F("repchr")); return *this; } @@ -299,7 +293,7 @@ public: } void copyto(char * const dst) const { strcpy(dst, str); } - void copyto(char * const dst, int len) const { strncpy(dst, str, len); } + void copyto(char * const dst, int len) const { strlcpy(dst, str, len + 1); } MString& clear() { return set(); } MString& eol() { return append('\n'); } @@ -318,6 +312,7 @@ public: #pragma GCC diagnostic pop +// Temporary inline string typically used to compose a G-code command #ifndef TS_SIZE #define TS_SIZE 63 #endif diff --git a/Marlin/src/gcode/queue.h b/Marlin/src/gcode/queue.h index aa7ef99f47..91cad1f08d 100644 --- a/Marlin/src/gcode/queue.h +++ b/Marlin/src/gcode/queue.h @@ -134,7 +134,7 @@ public: * Aborts the current SRAM queue so only use for one or two commands. */ static void inject(const char * const gcode) { - strncpy(injected_commands, gcode, sizeof(injected_commands) - 1); + strlcpy(injected_commands, gcode, sizeof(injected_commands)); } /** diff --git a/Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp b/Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp index df0c4df30d..63b1ef9c19 100644 --- a/Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp +++ b/Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp @@ -433,12 +433,12 @@ void ChironTFT::sendFileList(int8_t startindex) { } void ChironTFT::selectFile() { - const size_t namelen = command_len - 4 + (panel_type <= AC_panel_new); - strncpy(selectedfile, panel_command + 4, namelen); - selectedfile[namelen] = '\0'; + const size_t fnlen = command_len - 4 + (panel_type <= AC_panel_new); + strlcpy(selectedfile, panel_command + 4, fnlen + 1); #if ACDEBUG(AC_FILE) DEBUG_ECHOLNPGM(" Selected File: ", selectedfile); #endif + switch (selectedfile[0]) { case '/': // Valid file selected tftSendLn(AC_msg_sd_file_open_success); @@ -449,10 +449,9 @@ void ChironTFT::selectFile() { tftSendLn(AC_msg_sd_file_open_failed); sendFileList( 0 ); break; - default: // enter sub folder - // for new panel remove the '.GCO' tag that was added to the end of the path - if (panel_type <= AC_panel_new) - selectedfile[strlen(selectedfile) - 4] = '\0'; + default: // enter subfolder + // For new panel remove the '.GCO' tag that was added to the end of the path + if (panel_type <= AC_panel_new) selectedfile[fnlen - 4] = '\0'; filenavigator.changeDIR(selectedfile); tftSendLn(AC_msg_sd_file_open_failed); sendFileList( 0 ); diff --git a/Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp b/Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp index 4726cba378..3209aa76f2 100644 --- a/Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp +++ b/Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp @@ -971,8 +971,7 @@ namespace Anycubic { } void DgusTFT::selectFile() { - strncpy(selectedfile, panel_command + 4, command_len - 4); - selectedfile[command_len - 5] = '\0'; + strlcpy(selectedfile, panel_command + 4, command_len - 3); #if ACDEBUG(AC_FILE) DEBUG_ECHOLNPGM(" Selected File: ", selectedfile); #endif @@ -1293,8 +1292,7 @@ namespace Anycubic { TERN_(CASE_LIGHT_ENABLE, setCaseLightState(true)); char str_buf[20]; - strncpy_P(str_buf, filenavigator.filelist.longFilename(), 17); - str_buf[17] = '\0'; + strlcpy_P(str_buf, filenavigator.filelist.longFilename(), 18); sendTxtToTFT(str_buf, TXT_PRINT_NAME); #if ENABLED(POWER_LOSS_RECOVERY) @@ -1332,8 +1330,7 @@ namespace Anycubic { printFile(filenavigator.filelist.shortFilename()); char str_buf[20]; - strncpy_P(str_buf, filenavigator.filelist.longFilename(), 17); - str_buf[17] = '\0'; + strlcpy_P(str_buf, filenavigator.filelist.longFilename(), 18); sendTxtToTFT(str_buf, TXT_PRINT_NAME); sprintf(str_buf, "%5.2f", getFeedrate_percent()); diff --git a/Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp b/Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp index 90b181d6b5..ec7e549c41 100644 --- a/Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp +++ b/Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp @@ -155,7 +155,7 @@ static void lv_kb_event_cb(lv_obj_t *kb, lv_event_t event) { #endif // MKS_WIFI_MODULE case autoLevelGcodeCommand: uint8_t buf[100]; - strncpy((char *)buf, ret_ta_txt, sizeof(buf)); + strlcpy((char *)buf, ret_ta_txt, sizeof(buf)); update_gcode_command(AUTO_LEVELING_COMMAND_ADDR, buf); goto_previous_ui(); break; diff --git a/Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp b/Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp index da79cb6174..7732d5d2a4 100644 --- a/Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp +++ b/Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp @@ -474,7 +474,7 @@ void cutFileName(char *path, int len, int bytePerLine, char *outStr) { wcscpy(outStr, beginIndex); #else if ((int)strlen(beginIndex) > len) - strncpy(outStr, beginIndex, len); + strlcpy(outStr, beginIndex, len + 1); else strcpy(outStr, beginIndex); #endif @@ -485,17 +485,17 @@ void cutFileName(char *path, int len, int bytePerLine, char *outStr) { wcsncpy(outStr, (const WCHAR *)beginIndex, len - 3); wcscat(outStr, (const WCHAR *)gFileTail); #else - //strncpy(outStr, beginIndex, len - 3); - strncpy(outStr, beginIndex, len - 4); + strlcpy(outStr, beginIndex, len - 3); strcat_P(outStr, PSTR("~.g")); #endif } else { + const size_t strsize = strIndex2 - beginIndex + 1; #if _LFN_UNICODE - wcsncpy(outStr, (const WCHAR *)beginIndex, strIndex2 - beginIndex + 1); + wcsncpy(outStr, (const WCHAR *)beginIndex, strsize); wcscat(outStr, (const WCHAR *)&gFileTail[3]); #else - strncpy(outStr, beginIndex, strIndex2 - beginIndex + 1); + strlcpy(outStr, beginIndex, strsize + 1); strcat_P(outStr, PSTR("g")); #endif } diff --git a/Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp b/Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp index a759f8677e..eb9cac641a 100644 --- a/Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp +++ b/Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp @@ -727,8 +727,7 @@ void disp_assets_update_progress(FSTR_P const fmsg) { static constexpr int buflen = 30; char buf[buflen]; memset(buf, ' ', buflen); - strncpy_P(buf, FTOP(fmsg), buflen - 1); - buf[buflen - 1] = '\0'; + strlcpy_P(buf, FTOP(fmsg), buflen); disp_string(100, 165, buf, 0xFFFF, 0x0000); #else disp_string(100, 165, FTOP(fmsg), 0xFFFF, 0x0000); diff --git a/Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp b/Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp index ba898162d1..be63945143 100644 --- a/Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp +++ b/Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp @@ -237,11 +237,11 @@ void tft_lvgl_init() { uiCfg.print_state = REPRINTING; #if ENABLED(LONG_FILENAME_HOST_SUPPORT) - strncpy(public_buf_m, recovery.info.sd_filename, sizeof(public_buf_m)); + strlcpy(public_buf_m, recovery.info.sd_filename, sizeof(public_buf_m)); card.printLongPath(public_buf_m); - strncpy(list_file.long_name[sel_id], card.longFilename, sizeof(list_file.long_name[0])); + strlcpy(list_file.long_name[sel_id], card.longFilename, sizeof(list_file.long_name[0])); #else - strncpy(list_file.long_name[sel_id], recovery.info.sd_filename, sizeof(list_file.long_name[0])); + strlcpy(list_file.long_name[sel_id], recovery.info.sd_filename, sizeof(list_file.long_name[0])); #endif lv_draw_printing(); } diff --git a/Marlin/src/lcd/extui/nextion/nextion_tft.cpp b/Marlin/src/lcd/extui/nextion/nextion_tft.cpp index 87a6544e5e..4833ab96ab 100644 --- a/Marlin/src/lcd/extui/nextion/nextion_tft.cpp +++ b/Marlin/src/lcd/extui/nextion/nextion_tft.cpp @@ -158,8 +158,7 @@ void NextionTFT::sendFileList(int8_t startindex) { } void NextionTFT::selectFile() { - strncpy(selectedfile, nextion_command + 4, command_len - 4); - selectedfile[command_len - 5] = '\0'; + strlcpy(selectedfile, nextion_command + 4, command_len - 3); #if NEXDEBUG(N_FILE) DEBUG_ECHOLNPGM(" Selected File: ", selectedfile); #endif diff --git a/Marlin/src/lcd/lcdprint.cpp b/Marlin/src/lcd/lcdprint.cpp index f559272f7e..ea833f53a1 100644 --- a/Marlin/src/lcd/lcdprint.cpp +++ b/Marlin/src/lcd/lcdprint.cpp @@ -67,24 +67,24 @@ lcd_uint_t expand_u8str_P(char * const outstr, PGM_P const ptpl, const int8_t in } else { PGM_P const b = ind == -2 ? GET_TEXT(MSG_CHAMBER) : GET_TEXT(MSG_BED); - strncpy_P(o, b, n); + strlcpy_P(o, b, n + 1); n -= utf8_strlen(o); o += strlen(o); } if (n > 0) { - strncpy_P(o, (PGM_P)p, n); + strlcpy_P(o, (PGM_P)p, n + 1); n -= utf8_strlen(o); o += strlen(o); break; } } else if (wc == '$' && fstr) { - strncpy_P(o, FTOP(fstr), n); - n -= utf8_strlen_P(FTOP(fstr)); + strlcpy_P(o, FTOP(fstr), n + 1); + n -= utf8_strlen(o); o += strlen(o); } else if (wc == '$' && cstr) { - strncpy(o, cstr, n); + strlcpy(o, cstr, n + 1); n -= utf8_strlen(o); o += strlen(o); } diff --git a/Marlin/src/lcd/menu/menu_item.h b/Marlin/src/lcd/menu/menu_item.h index 11f78d25d5..823d2a4a25 100644 --- a/Marlin/src/lcd/menu/menu_item.h +++ b/Marlin/src/lcd/menu/menu_item.h @@ -386,11 +386,11 @@ class MenuItem_bool : public MenuEditItemBase { #define PSTRING_ITEM_F_P(FLABEL, PVAL, STYL) do{ \ constexpr int m = 20; \ - char msg[m+1]; \ + char msg[m + 1]; \ if (_menuLineNr == _thisItemNr) { \ msg[0] = ':'; msg[1] = ' '; \ - strncpy_P(msg+2, PVAL, m-2); \ - if (msg[m-1] & 0x80) msg[m-1] = '\0'; \ + strlcpy_P(msg + 2, PVAL, m - 1); \ + if (msg[m - 1] & 0x80) msg[m - 1] = '\0'; \ } \ STATIC_ITEM_F(FLABEL, STYL, msg); \ }while(0) diff --git a/Marlin/src/module/settings.cpp b/Marlin/src/module/settings.cpp index 6f7c4b1ebc..f636b985e1 100644 --- a/Marlin/src/module/settings.cpp +++ b/Marlin/src/module/settings.cpp @@ -2042,7 +2042,7 @@ void MarlinSettings::postprocess() { if (grid_max_x == (GRID_MAX_POINTS_X) && grid_max_y == (GRID_MAX_POINTS_Y)) { if (!validating) set_bed_leveling_enabled(false); bedlevel.set_grid(spacing, start); - EEPROM_READ(bedlevel.z_values); // 9 to 256 floats + EEPROM_READ(bedlevel.z_values); // 9 to 256 floats } else if (grid_max_x > (GRID_MAX_POINTS_X) || grid_max_y > (GRID_MAX_POINTS_Y)) { eeprom_error = ERR_EEPROM_CORRUPT; diff --git a/Marlin/src/sd/cardreader.cpp b/Marlin/src/sd/cardreader.cpp index 9b9b3509d6..175e4e5c05 100644 --- a/Marlin/src/sd/cardreader.cpp +++ b/Marlin/src/sd/cardreader.cpp @@ -446,8 +446,7 @@ void CardReader::ls(const uint8_t lsflags/*=0*/) { diveDir.close(); if (longFilename[0]) { - strncpy_P(pathLong, longFilename, 63); - pathLong[63] = '\0'; + strlcpy_P(pathLong, longFilename, 64); break; } } @@ -1075,8 +1074,7 @@ const char* CardReader::diveToFile(const bool update_cwd, MediaFile* &inDirPtr, // Isolate the next subitem name const uint8_t len = name_end - atom_ptr; char dosSubdirname[len + 1]; - strncpy(dosSubdirname, atom_ptr, len); - dosSubdirname[len] = 0; + strlcpy(dosSubdirname, atom_ptr, len + 1); if (echo) SERIAL_ECHOLN(dosSubdirname); @@ -1181,7 +1179,7 @@ void CardReader::cdroot() { #endif #else // Copy filenames into the static array - #define _SET_SORTNAME(I) strncpy(sortnames[I], longest_filename(), SORTED_LONGNAME_MAXLEN) + #define _SET_SORTNAME(I) strlcpy(sortnames[I], longest_filename(), sizeof(sortnames[I])) #if SORTED_LONGNAME_MAXLEN == LONG_FILENAME_LENGTH // Short name sorting always use LONG_FILENAME_LENGTH with no trailing nul #define SET_SORTNAME(I) _SET_SORTNAME(I)