fixed -Wformat-overflow compiler warnings / added missing newline to log messages

[firewave]

Some examples for reference:

Src/OSD/SDL/Main.cpp:1086:30: warning: ‘ (Paused)’ directive writing 9 bytes into a region of size between 1 and 128 [-Wformat-overflow=]
 1086 |         sprintf(titleStr, "%s (Paused)", baseTitleStr);
      |                              ^~~~~~~~~
Src/OSD/SDL/Main.cpp:1086:16: note: ‘sprintf’ output between 10 and 137 bytes into a destination of size 128
 1086 |         sprintf(titleStr, "%s (Paused)", baseTitleStr);
      |         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Src/OSD/Logger.cpp:382:29: warning: ‘%s’ directive writing up to 4095 bytes into a region of size 4088 [-Wformat-overflow=]
  382 |   sprintf(string2, "[Error] %s\n", string1);
      |                             ^~     ~~~~~~~
Src/OSD/Logger.cpp:382:10: note: ‘sprintf’ output between 10 and 4105 bytes into a destination of size 4096
  382 |   sprintf(string2, "[Error] %s\n", string1);
      |   ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[trzy]

Please see the attached comments. I don't think the buffer lengths should be touched in Logger.cpp, there is no need for it. Rather, use the safe forms of vsprintf and sprintf (vsnprintf and snprintf). Also, the \n in debug logging was intentionally omitted. It's not a typo.

[trzy]

These are not needed. 4096 was chosen somewhat arbitrarily. We don't actually check the size of the resulting output. If you want to fix this, you should instead prefer to use vsnprintf() and pass 4096 as the "n" parameter. Likewise, sprintf -> snprintf.

[firewave]

Okay. I moved back to the original buffer length and use *snprintf() now.

[trzy]

This newline is actually incorrect. DebugLog is special and you'll notice wherever it is used in Supermodel, a \n is manually included. The intent behind this was to allow formatting debug output more freely with multiple calls to generate a single line if needed. However, now that the [Debug] tag has been added, it may be worthwhile to automatically include a \n but if this is done, all places where DebugLog is used must be updated to drop their \n.

[firewave]

Done.

[trzy]

Same here.

[firewave]

Done.

[trzy]

This change is fine, I suppose. Does anything actually exceed 128 bytes? We should probably also move to using snprintf() for safety here.

[firewave]

As it appends up to 128 bytes from baseTitleStr in that string it needs to be at least 128 + n.

Src/OSD/SDL/Main.cpp:1086:30: warning: ‘ (Paused)’ directive writing 9 bytes into a region of size between 1 and 128 [-Wformat-overflow=]
 1086 |         sprintf(titleStr, "%s (Paused)", baseTitleStr);
      |                              ^~~~~~~~~
Src/OSD/SDL/Main.cpp:1086:16: note: ‘sprintf’ output between 10 and 137 bytes into a destination of size 128
 1086 |         sprintf(titleStr, "%s (Paused)", baseTitleStr);
      |         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[trzy]

Will you be able to make the suggested changes?

[firewave]

Will you be able to make the suggested changes?

Yes. Hopefully I finally get to it this week.

[firewave]

There are additional warnings now but I cannot change that file because there are some issues with the line endings.

Src/Debugger/CPU/68KDebug.cpp:1353:56: note: ‘sprintf’ output between 15 and 64 bytes into a destination of size 50
 1353 |                                                 sprintf(moveStr, "movem.%c %s,[AM2]", sizeC, regList);
      |                                                 ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~