From 63f7add00c3f9a74231b6f39a67b4aeb9190476e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Jos=C3=A9=20Tagle?= Date: Sat, 26 May 2018 01:31:19 -0300 Subject: [PATCH] [1.1.x] Buffer overflow and scroll fix, UTF8 cleanup (#10851) --- Marlin/G26_Mesh_Validation_Tool.cpp | 5 -- Marlin/Marlin_main.cpp | 5 +- Marlin/status_screen_DOGM.h | 85 ++++++++++++++----- Marlin/status_screen_lite_ST7920.h | 75 ++++++++++++----- Marlin/ubl.h | 1 - Marlin/ultralcd.cpp | 72 +++++++++------- Marlin/ultralcd_impl_HD44780.h | 125 ++++++++++++++++++++++------ Marlin/utf_mapper.h | 2 +- 8 files changed, 263 insertions(+), 107 deletions(-) diff --git a/Marlin/G26_Mesh_Validation_Tool.cpp b/Marlin/G26_Mesh_Validation_Tool.cpp index ae6c93b20..8a3f951c8 100644 --- a/Marlin/G26_Mesh_Validation_Tool.cpp +++ b/Marlin/G26_Mesh_Validation_Tool.cpp @@ -134,9 +134,6 @@ // External references extern Planner planner; - #if ENABLED(ULTRA_LCD) - extern char lcd_status_message[]; - #endif // Private functions @@ -274,8 +271,6 @@ wait_for_release(); - strcpy_P(lcd_status_message, PSTR("Done Priming")); // Hack to get the message up. May be obsolete. - lcd_setstatusPGM(PSTR("Done Priming"), 99); lcd_quick_feedback(true); lcd_external_control = false; diff --git a/Marlin/Marlin_main.cpp b/Marlin/Marlin_main.cpp index c76201915..1ccf7ff5c 100644 --- a/Marlin/Marlin_main.cpp +++ b/Marlin/Marlin_main.cpp @@ -5894,15 +5894,12 @@ void home_all_axes() { gcode_G28(true); } } // Report settings - const char *checkingac = PSTR("Checking... AC"); serialprintPGM(checkingac); if (verbose_level == 0) SERIAL_PROTOCOLPGM(" (DRY-RUN)"); if (set_up) SERIAL_PROTOCOLPGM(" (SET-UP)"); SERIAL_EOL(); - char mess[11]; - strcpy_P(mess, checkingac); - lcd_setstatus(mess); + lcd_setstatusPGM(checkingac); print_calibration_settings(_endstop_results, _angle_results); diff --git a/Marlin/status_screen_DOGM.h b/Marlin/status_screen_DOGM.h index b718b6217..5faeb6e40 100644 --- a/Marlin/status_screen_DOGM.h +++ b/Marlin/status_screen_DOGM.h @@ -124,36 +124,77 @@ FORCE_INLINE void _draw_axis_value(const AxisEnum axis, const char *value, const inline void lcd_implementation_status_message(const bool blink) { #if ENABLED(STATUS_MESSAGE_SCROLLING) static bool last_blink = false; - const uint8_t slen = lcd_strlen(lcd_status_message); - const char *stat = lcd_status_message + status_scroll_pos; - if (slen <= LCD_WIDTH) - lcd_print_utf(stat); // The string isn't scrolling + + // Get the UTF8 character count of the string + uint8_t slen = lcd_strlen(lcd_status_message); + + // If the string fits into the LCD, just print it and do not scroll it + if (slen <= LCD_WIDTH) { + + // The string isn't scrolling and may not fill the screen + lcd_print_utf(lcd_status_message); + + // Fill the rest with spaces + while (slen < LCD_WIDTH) { + u8g.print(' '); + ++slen; + } + } else { - if (status_scroll_pos <= slen - LCD_WIDTH) - lcd_print_utf(stat); // The string fills the screen + // String is larger than the available space in screen. + + // Get a pointer to the next valid UTF8 character + const char *stat = lcd_status_message + status_scroll_offset; + + // Get the string remaining length + const uint8_t rlen = lcd_strlen(stat); + + // If we have enough characters to display + if (rlen >= LCD_WIDTH) { + // The remaining string fills the screen - Print it + lcd_print_utf(stat, LCD_WIDTH); + } else { - uint8_t chars = LCD_WIDTH; - if (status_scroll_pos < slen) { // First string still visible - lcd_print_utf(stat); // The string leaves space - chars -= slen - status_scroll_pos; // Amount of space left - } - u8g.print('.'); // Always at 1+ spaces left, draw a dot - if (--chars) { - if (status_scroll_pos < slen + 1) // Draw a second dot if there's space - --chars, u8g.print('.'); - if (chars) lcd_print_utf(lcd_status_message, chars); // Print a second copy of the message + // The remaining string does not completely fill the screen + lcd_print_utf(stat, LCD_WIDTH); // The string leaves space + uint8_t chars = LCD_WIDTH - rlen; // Amount of space left in characters + + u8g.print('.'); // Always at 1+ spaces left, draw a dot + if (--chars) { // Draw a second dot if there's space + u8g.print('.'); + if (--chars) { + // Print a second copy of the message + lcd_print_utf(lcd_status_message, LCD_WIDTH - (rlen+2)); + } } } - if (last_blink != blink) { - last_blink = blink; - // Skip any non-printing bytes - if (status_scroll_pos < slen) while (!PRINTABLE(lcd_status_message[status_scroll_pos])) status_scroll_pos++; - if (++status_scroll_pos >= slen + 2) status_scroll_pos = 0; + if (last_blink != blink) { + last_blink = blink; + + // Adjust by complete UTF8 characters + if (status_scroll_offset < slen) { + status_scroll_offset++; + while (!START_OF_UTF8_CHAR(lcd_status_message[status_scroll_offset])) + status_scroll_offset++; + } + else + status_scroll_offset = 0; } } #else UNUSED(blink); - lcd_print_utf(lcd_status_message); + + // Get the UTF8 character count of the string + uint8_t slen = lcd_strlen(lcd_status_message); + + // Just print the string to the LCD + lcd_print_utf(lcd_status_message, LCD_WIDTH); + + // Fill the rest with spaces if there are missing spaces + while (slen < LCD_WIDTH) { + u8g.print(' '); + ++slen; + } #endif } diff --git a/Marlin/status_screen_lite_ST7920.h b/Marlin/status_screen_lite_ST7920.h index 987aba4f7..37eabe14b 100644 --- a/Marlin/status_screen_lite_ST7920.h +++ b/Marlin/status_screen_lite_ST7920.h @@ -615,36 +615,71 @@ void ST7920_Lite_Status_Screen::draw_feedrate_percentage(const uint8_t percentag void ST7920_Lite_Status_Screen::draw_status_message(const char *str) { set_ddram_address(DDRAM_LINE_4); begin_data(); + const uint8_t lcd_len = 16; #if ENABLED(STATUS_MESSAGE_SCROLLING) - const uint8_t lcd_len = 16; - const uint8_t padding = 2; - uint8_t str_len = strlen(str); + + uint8_t slen = lcd_strlen(str); - // Trim whitespace at the end of the str, as for some reason - // messages like "Card Inserted" are padded with many spaces - while (str_len && str[str_len - 1] == ' ') str_len--; + // If the string fits into the LCD, just print it and do not scroll it + if (slen <= lcd_len) { - if (str_len <= lcd_len) { - // It all fits on the LCD without scrolling + // The string isn't scrolling and may not fill the screen write_str(str); + + // Fill the rest with spaces + while (slen < lcd_len) { + write_byte(' '); + ++slen; + } } else { - // Print the message repeatedly until covering the LCD - uint8_t c = status_scroll_pos; - for (uint8_t n = 0; n < lcd_len; n++) { - write_byte(c < str_len ? str[c] : ' '); - c++; - c %= str_len + padding; // Wrap around + // String is larger than the available space in screen. + + // Get a pointer to the next valid UTF8 character + const char *stat = str + status_scroll_offset; + + // Get the string remaining length + const uint8_t rlen = lcd_strlen(stat); + + // If we have enough characters to display + if (rlen >= lcd_len) { + // The remaining string fills the screen - Print it + write_str(stat, lcd_len); + } + else { + // The remaining string does not completely fill the screen + write_str(stat); // The string leaves space + uint8_t chars = lcd_len - rlen; // Amount of space left in characters + + write_byte('.'); // Always at 1+ spaces left, draw a dot + if (--chars) { // Draw a second dot if there's space + write_byte('.'); + if (--chars) + write_str(str, chars); // Print a second copy of the message + } } - // Scroll the message - if (status_scroll_pos == str_len + padding) - status_scroll_pos = 0; + // Adjust by complete UTF8 characters + if (status_scroll_offset < slen) { + status_scroll_offset++; + while (!START_OF_UTF8_CHAR(str[status_scroll_offset])) + status_scroll_offset++; + } else - status_scroll_pos++; + status_scroll_offset = 0; } #else - write_str(str, 16); + // Get the UTF8 character count of the string + uint8_t slen = lcd_strlen(str); + + // Just print the string to the LCD + write_str(str, lcd_len); + + // Fill the rest with spaces if there are missing spaces + while (slen < lcd_len) { + write_byte(' '); + ++slen; + } #endif } @@ -792,7 +827,7 @@ void ST7920_Lite_Status_Screen::update_status_or_position(bool forceUpdate) { */ if (forceUpdate || status_changed()) { #if ENABLED(STATUS_MESSAGE_SCROLLING) - status_scroll_pos = 0; + status_scroll_offset = 0; #endif #if STATUS_EXPIRE_SECONDS countdown = lcd_status_message[0] ? STATUS_EXPIRE_SECONDS : 0; diff --git a/Marlin/ubl.h b/Marlin/ubl.h index cc22f80bd..7b9c08fe6 100644 --- a/Marlin/ubl.h +++ b/Marlin/ubl.h @@ -61,7 +61,6 @@ extern uint8_t ubl_cnt; /////////////////////////////////////////////////////////////////////////////////////////////////////// #if ENABLED(ULTRA_LCD) - extern char lcd_status_message[]; void lcd_quick_feedback(const bool clear_buttons); #endif diff --git a/Marlin/ultralcd.cpp b/Marlin/ultralcd.cpp index 81343f02d..6dc2d3677 100644 --- a/Marlin/ultralcd.cpp +++ b/Marlin/ultralcd.cpp @@ -71,7 +71,7 @@ #else #define MAX_MESSAGE_LENGTH CHARSIZE * 2 * (LCD_WIDTH) #endif - uint8_t status_scroll_pos = 0; + uint8_t status_scroll_offset = 0; #else #define MAX_MESSAGE_LENGTH CHARSIZE * (LCD_WIDTH) #endif @@ -5030,7 +5030,7 @@ void lcd_init() { int16_t lcd_strlen(const char* s) { int16_t i = 0, j = 0; while (s[i]) { - if (PRINTABLE(s[i])) j++; + if (START_OF_UTF8_CHAR(s[i])) j++; i++; } return j; @@ -5039,7 +5039,7 @@ int16_t lcd_strlen(const char* s) { int16_t lcd_strlen_P(const char* s) { int16_t j = 0; while (pgm_read_byte(s)) { - if (PRINTABLE(pgm_read_byte(s))) j++; + if (START_OF_UTF8_CHAR(pgm_read_byte(s))) j++; s++; } return j; @@ -5367,30 +5367,8 @@ void lcd_update() { } // ELAPSED(ms, next_lcd_update_ms) } -inline void pad_message_string() { - uint8_t i = 0, j = 0; - char c; - lcd_status_message[MAX_MESSAGE_LENGTH] = '\0'; - while ((c = lcd_status_message[i]) && j < LCD_WIDTH) { - if (PRINTABLE(c)) j++; - i++; - } - if (true - #if ENABLED(STATUS_MESSAGE_SCROLLING) - && j < LCD_WIDTH - #endif - ) { - // pad with spaces to fill up the line - while (j++ < LCD_WIDTH) lcd_status_message[i++] = ' '; - // chop off at the edge - lcd_status_message[i] = '\0'; - } -} - void lcd_finishstatus(const bool persist=false) { - pad_message_string(); - #if !(ENABLED(LCD_PROGRESS_BAR) && (PROGRESS_MSG_EXPIRE > 0)) UNUSED(persist); #endif @@ -5408,7 +5386,7 @@ void lcd_finishstatus(const bool persist=false) { #endif #if ENABLED(STATUS_MESSAGE_SCROLLING) - status_scroll_pos = 0; + status_scroll_offset = 0; #endif } @@ -5420,7 +5398,26 @@ bool lcd_hasstatus() { return (lcd_status_message[0] != '\0'); } void lcd_setstatus(const char * const message, const bool persist) { if (lcd_status_message_level > 0) return; - strncpy(lcd_status_message, message, MAX_MESSAGE_LENGTH); + + // Here we have a problem. The message is encoded in UTF8, so + // arbitrarily cutting it will be a problem. We MUST be sure + // that there is no cutting in the middle of a multibyte character! + + // Get a pointer to the null terminator + const char* pend = message + strlen(message); + + // If length of supplied UTF8 string is greater than + // our buffer size, start cutting whole UTF8 chars + while ((pend - message) > MAX_MESSAGE_LENGTH) { + --pend; + while (!START_OF_UTF8_CHAR(*pend)) --pend; + }; + + // At this point, we have the proper cut point. Use it + uint8_t maxLen = pend - message; + strncpy(lcd_status_message, message, maxLen); + lcd_status_message[maxLen] = '\0'; + lcd_finishstatus(persist); } @@ -5428,7 +5425,26 @@ void lcd_setstatusPGM(const char * const message, int8_t level) { if (level < 0) level = lcd_status_message_level = 0; if (level < lcd_status_message_level) return; lcd_status_message_level = level; - strncpy_P(lcd_status_message, message, MAX_MESSAGE_LENGTH); + + // Here we have a problem. The message is encoded in UTF8, so + // arbitrarily cutting it will be a problem. We MUST be sure + // that there is no cutting in the middle of a multibyte character! + + // Get a pointer to the null terminator + const char* pend = message + strlen_P(message); + + // If length of supplied UTF8 string is greater than + // our buffer size, start cutting whole UTF8 chars + while ((pend - message) > MAX_MESSAGE_LENGTH) { + --pend; + while (!START_OF_UTF8_CHAR(pgm_read_byte(pend))) --pend; + }; + + // At this point, we have the proper cut point. Use it + uint8_t maxLen = pend - message; + strncpy_P(lcd_status_message, message, maxLen); + lcd_status_message[maxLen] = '\0'; + lcd_finishstatus(level > 0); } diff --git a/Marlin/ultralcd_impl_HD44780.h b/Marlin/ultralcd_impl_HD44780.h index 1c35348df..0926c06e4 100644 --- a/Marlin/ultralcd_impl_HD44780.h +++ b/Marlin/ultralcd_impl_HD44780.h @@ -491,13 +491,42 @@ void lcd_printPGM_utf(const char *str, uint8_t n=LCD_WIDTH) { // Scroll the PSTR 'text' in a 'len' wide field for 'time' milliseconds at position col,line void lcd_scroll(const int16_t col, const int16_t line, const char* const text, const int16_t len, const int16_t time) { - char tmp[LCD_WIDTH + 1] = {0}; - const int16_t n = MAX(lcd_strlen_P(text) - len, 0); - for (int16_t i = 0; i <= n; i++) { - strncpy_P(tmp, text + i, MIN(len, LCD_WIDTH)); + uint8_t slen = lcd_strlen_P(text); + if (slen < len) { + // Fits into, lcd.setCursor(col, line); - lcd_print(tmp); - delay(time / MAX(n, 1)); + lcd_printPGM_utf(text, len); + while (slen < len) { + lcd.write(' '); + ++slen; + } + safe_delay(time); + } + else { + const char* p = text; + int dly = time / MAX(slen, 1); + for (uint8_t i = 0; i <= slen; i++) { + + // Go to the correct place + lcd.setCursor(col, line); + + // Print the text + lcd_printPGM_utf(p, len); + + // Fill with spaces + uint8_t ix = slen - i; + while (ix < len) { + lcd.write(' '); + ++ix; + } + + // Delay + safe_delay(dly); + + // Advance to the next UTF8 valid position + p++; + while (!START_OF_UTF8_CHAR(pgm_read_byte(p))) p++; + } } } @@ -895,38 +924,82 @@ static void lcd_implementation_status_screen() { #if ENABLED(STATUS_MESSAGE_SCROLLING) static bool last_blink = false; - const uint8_t slen = lcd_strlen(lcd_status_message); - const char *stat = lcd_status_message + status_scroll_pos; - if (slen <= LCD_WIDTH) - lcd_print_utf(stat); // The string isn't scrolling + + // Get the UTF8 character count of the string + uint8_t slen = lcd_strlen(lcd_status_message); + + // If the string fits into the LCD, just print it and do not scroll it + if (slen <= LCD_WIDTH) { + + // The string isn't scrolling and may not fill the screen + lcd_print_utf(lcd_status_message); + + // Fill the rest with spaces + while (slen < LCD_WIDTH) { + lcd.write(' '); + ++slen; + } + } else { - if (status_scroll_pos <= slen - LCD_WIDTH) - lcd_print_utf(stat); // The string fills the screen + // String is larger than the available space in screen. + + // Get a pointer to the next valid UTF8 character + const char *stat = lcd_status_message + status_scroll_offset; + + // Get the string remaining length + const uint8_t rlen = lcd_strlen(stat); + + // If we have enough characters to display + if (rlen >= LCD_WIDTH) { + // The remaining string fills the screen - Print it + lcd_print_utf(stat, LCD_WIDTH); + } else { - uint8_t chars = LCD_WIDTH; - if (status_scroll_pos < slen) { // First string still visible - lcd_print_utf(stat); // The string leaves space - chars -= slen - status_scroll_pos; // Amount of space left - } - lcd.write('.'); // Always at 1+ spaces left, draw a dot - if (--chars) { - if (status_scroll_pos < slen + 1) // Draw a second dot if there's space - --chars, lcd.write('.'); - if (chars) lcd_print_utf(lcd_status_message, chars); // Print a second copy of the message + + // The remaining string does not completely fill the screen + lcd_print_utf(stat, LCD_WIDTH); // The string leaves space + uint8_t chars = LCD_WIDTH - rlen; // Amount of space left in characters + + lcd.write('.'); // Always at 1+ spaces left, draw a dot + if (--chars) { // Draw a second dot if there's space + lcd.write('.'); + if (--chars) + lcd_print_utf(lcd_status_message, chars); // Print a second copy of the message } } if (last_blink != blink) { last_blink = blink; - // Skip any non-printing bytes - if (status_scroll_pos < slen) while (!PRINTABLE(lcd_status_message[status_scroll_pos])) status_scroll_pos++; - if (++status_scroll_pos >= slen + 2) status_scroll_pos = 0; + + // Adjust by complete UTF8 characters + if (status_scroll_offset < slen) { + status_scroll_offset++; + while (!START_OF_UTF8_CHAR(lcd_status_message[status_scroll_offset])) + status_scroll_offset++; + } + else + status_scroll_offset = 0; } } #else - lcd_print_utf(lcd_status_message); + UNUSED(blink); + + // Get the UTF8 character count of the string + uint8_t slen = lcd_strlen(lcd_status_message); + + // Just print the string to the LCD + lcd_print_utf(lcd_status_message, LCD_WIDTH); + + // Fill the rest with spaces if there are missing spaces + while (slen < LCD_WIDTH) { + lcd.write(' '); + ++slen; + } #endif + } + + #if ENABLED(ULTIPANEL) #if ENABLED(ADVANCED_PAUSE_FEATURE) diff --git a/Marlin/utf_mapper.h b/Marlin/utf_mapper.h index c49e6fc4e..aacf2f11d 100644 --- a/Marlin/utf_mapper.h +++ b/Marlin/utf_mapper.h @@ -144,7 +144,7 @@ #endif // DISPLAY_CHARSET_HD44780 #endif // SIMULATE_ROMFONT -#define PRINTABLE(C) (((C) & 0xC0u) != 0x80u) +#define START_OF_UTF8_CHAR(C) (((C) & 0xC0u) != 0x80u) #if ENABLED(MAPPER_C2C3)