[2.0.x] Buffer overflow fixes and UTF8 cleanup

[ejtagle]

As said, G26 command processing had a buffer overflow.
Also removed some unneeded hacks

Clean up of UTF8 handling and possible buffer overflows: It must be clear that the UTF8 representation of characters can use more than 1 byte per character. So, for example, strlen(buffer) will always return the count of BYTES (not characters) required to store an UTF8 string. strcpy and strcat works.
But, strncpy() has a problem, as the count passed to that function is in bytes, not in characters. That could lead to truncation in the middle of a multibyte UTF8 character, and then and invalid UTF8 character is stored.
The routines used (get_utf8_value_cb) to fetch UTF8 chars do not validate that, thus, the end of the UTF8 string will not be detected.
So, it is of paramount importance to handle truncation in a proper way. This PR addresses that.

Also, removed unnecesary functions and simplified the UTF8 handling a bit.

Finally, the bug that was present when STATUS_MESSAGE_SCROLLING was defined (not proper estimation of scroll positions (UTF8 is multibyte) and the text was larger than the screen, was also fixed

I tried it on a Graphical display and it works. It would be nice if someone tries it in a text display 👍

[thinkyhead]

Overall, looks good. I can't tell if the message string is still being padded out with spaces. That's important to make sure a previous longer message is erased by a new shorter message. I don't have my character display on hand at the moment, but I can test that tomorrow.

Under the current policy, once you make an equivalent PR for bugfix-1.1.x then they can both be merged. This PR will be held off until that's ready. You should be able to mostly copy, paste, and cherry-pick to put it together. Of course, the bugfix-1.1.x branch doesn't have a HAL or the new text engine, so those changes won't be needed.

[Roxy-3D]

@MagoKimbra says this broke things on his display. Are there any other people having problems with their LCD Panel (because of this commit) ?

[ejtagle]

@Roxy-3D : I did test 2.0.x with a graphical display. I have no way to test 1.1.x. If @MagoKimbra is kind enough to describe the problem (some pictures would be nice) i think it can be fixed (but, again, logically i see no problems)

[Roxy-3D]

I understand. Nobody else is saying anything, so let's just sit tight and wait for more information from MagoKimbra.

[MagoKimbra]

Yes on graphical display it's all ok... On testual display i have the problem...

[Roxy-3D]

Wow! That does look interesting!

[MagoKimbra]

I download firmware 2.0 default parameters change only G3D display... Arduino IDE 1.8.5 and i have all library. With the last commit it's all ok...
The character change on display very fast. The board result blocked if connect with HOST. After more second the board is reset I see the led flashing quickly ..
I think it's a writing problem in progmem going to clog up the processor memory ...

[ejtagle]

@MagoKimbra : Could you comment out the "lcd_scroll" function ? ... (i mean, just
#if 0 out the body, so it does nothing? - And check if that "fixes" the issues ?

[MagoKimbra]

Yes i try!!! on moment!

[MagoKimbra]

No the problem is same.

[ejtagle]

@MagoKimbra :
As you say, if the graphics display works, and the text display does not, it must be a change to the text display code.
There are 2 places that could be ... One is the status update, and the other one was the lcd_scroll.
Do you have enabled STATUS_MESSAGE_SCROLLING ?

If you look at ultralcd_impl_HD44780.h, there is a #if ENABLED(STATUS_MESSAGE_SCROLLING) block that is the other piece of code i changed.

You can also try either disabling the STATUS_MESSAGE_SCROLLING configuration, or commenting out that piece of code.

The idea is to isolate the change that is causing the problem

[ejtagle]

(the main problem is that i don´t have a HD44780 lcd attached to the printer... ;( ) -- Right now i am seriously considering in writing an emulator based on avrsim, so all those kinds of tests can be done by everybody without needing hardware

[MagoKimbra]

I'm slowly modifying the code by going back to the hd44780 files ...
Let's see if I find the problem.

[ejtagle]

@MagoKimbra : I think i found the problem!! ... Please, in file lcdprint_hd44780.cpp, look for the function lcd_put_u8str_max_cb. There it is a line that says:
if (!p) break;
Change it to
if (!ch) break;

[ejtagle]

I am not sure how that change slipped. I modified that function, in both the graphics display version and the text display version. But somehow i forgot to do that important change to the text version. Without it, the function is unable to detect the end of the string... :(

[MagoKimbra]

Ok i found the problem... In lcdprint_hd44780.cpp the routine lcd_put_u8str_max and lcd_put_u8str_max_P
With old routine it's ok.
Now search the problem.

[ejtagle]

@MagoKimbra : I posted the possible fix ...

in file lcdprint_hd44780.cpp, look for the function lcd_put_u8str_max_cb. There it is a line that says:
if (!p) break;
Change it to
if (!ch) break;

[MagoKimbra]

Ok now function...
The problem is in function lcd_put_u8str_max_cb

wchar_t ch = 0;
    p = get_utf8_value_cb(p, cb_read_byte, &ch);
    if (!p) break;
    ret += lcd_put_wchar_max(ch, max_length - ret);

in

wchar_t ch = 0;
    p = get_utf8_value_cb(p, cb_read_byte, &ch);
    if (!ch) break;
    ret += lcd_put_wchar_max(ch, max_length - ret);

[MagoKimbra]

Ops... you had the solution... Ok it's ok now...