Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Minor string optimization via strlcpy #26513

Merged
merged 1 commit into from
Dec 8, 2023
Merged

Minor string optimization via strlcpy #26513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Marlin/src/HAL/LINUX/include/Arduino.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@

#include <pinmapping.h>

#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

Expand Down
35 changes: 15 additions & 20 deletions Marlin/src/core/mstring.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -98,26 +98,20 @@ class MString {

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
}

void safety(const int n) { if (SAFE && n <= SIZE) str[n] = '\0'; }

// 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<char*>(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; }
Expand All @@ -134,11 +128,11 @@ class MString {
MString& set(const xyze_pos_t &v) { set(); return append(v); }

template <int S>
MString& set(const MString<S> &m) { strncpy(str, &m, SIZE); debug(F("MString")); return *this; }
MString& set(const MString<S> &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<char*>(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))
Expand All @@ -159,9 +153,9 @@ class MString {

// 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<char *>(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; }
Expand Down Expand Up @@ -195,15 +189,15 @@ class MString {
MString& append(const MString<S> &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<char *>(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;
}
Expand Down Expand Up @@ -299,7 +293,7 @@ class MString {
}

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'); }
Expand All @@ -318,6 +312,7 @@ class MString {

#pragma GCC diagnostic pop

// Temporary inline string typically used to compose a G-code command
#ifndef TS_SIZE
#define TS_SIZE 63
#endif
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/gcode/queue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class GCodeQueue {
* 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));
}

/**
Expand Down
13 changes: 6 additions & 7 deletions Marlin/src/lcd/extui/anycubic_chiron/chiron_tft.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 );
Expand Down
9 changes: 3 additions & 6 deletions Marlin/src/lcd/extui/anycubic_vyper/dgus_tft.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/lcd/extui/mks_ui/draw_keyboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions Marlin/src/lcd/extui/mks_ui/draw_print_file.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions Marlin/src/lcd/extui/mks_ui/mks_hardware.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions Marlin/src/lcd/extui/mks_ui/tft_lvgl_configuration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
3 changes: 1 addition & 2 deletions Marlin/src/lcd/extui/nextion/nextion_tft.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions Marlin/src/lcd/lcdprint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions Marlin/src/lcd/menu/menu_item.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/module/settings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions Marlin/src/sd/cardreader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand Down
Loading