The change introduced by [r7661] causes a subtle change to the return value of sprintf() on error.
We will have to decide which we want.
A quick review shows some code in using the return value incorrectly in either case.
I haven't seen anyplace where we actually check if the string was truncated.
I'm trying to fix potential buffer overflows of buffers for file names on linux or windows with long file names enabled. We use 256 all over the place for file name lengths which is even too short for windows. The conversions from sprintf to snprintf with larger buffers (1024) is to prevent memory corruption from buffer overflows. It doesn't address truncated file names but we should get file open errors rather than crashes when it happens.
This is just a short term fix for long file names. We should be using std::string but a lot of code would need to be changed because most core code expects pointers. It's hard to convert legacy C style C++ code to proper C++ all at once. We could do it a file at a time or a function at a time but it won't be easy. A lot of the core code should be converted from functions and global variables to objects and RAII. We should also use at least C++ 11 or newer and drop support for old compilers. I just want to get linux working with long file names now and worry about making it better later.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I haven't seen anyplace where we actually check if the string was truncated.
I haven't either, but there is some code that uses the return value. It appears to me that it would not work in the truncation case with either SD_snprintf() or the real snprintf(). It uses a 4096 byte buffer so it is likely not failing. I will look into it. [r7662] changes SD_snprintf() to be consistent with snprintf(), so at least all platforms will return the same value on error.
Previously, it would return -1 on truncation for MSVC, now it returns the needed buffer size (like most platforms and newer MSVC vesions)
I can now run speed-dreams-2 with 700 character long file names on linux without crashing or getting errors by removing a lot of the snprintfs. There are still many remaining but it's a start.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I haven't seen anyplace where we actually check if the string was truncated.
I'm trying to fix potential buffer overflows of buffers for file names on linux or windows with long file names enabled. We use 256 all over the place for file name lengths which is even too short for windows. The conversions from sprintf to snprintf with larger buffers (1024) is to prevent memory corruption from buffer overflows. It doesn't address truncated file names but we should get file open errors rather than crashes when it happens.
This is just a short term fix for long file names. We should be using std::string but a lot of code would need to be changed because most core code expects pointers. It's hard to convert legacy C style C++ code to proper C++ all at once. We could do it a file at a time or a function at a time but it won't be easy. A lot of the core code should be converted from functions and global variables to objects and RAII. We should also use at least C++ 11 or newer and drop support for old compilers. I just want to get linux working with long file names now and worry about making it better later.
I haven't either, but there is some code that uses the return value. It appears to me that it would not work in the truncation case with either SD_snprintf() or the real snprintf(). It uses a 4096 byte buffer so it is likely not failing. I will look into it.
[r7662] changes SD_snprintf() to be consistent with snprintf(), so at least all platforms will return the same value on error.
Previously, it would return -1 on truncation for MSVC, now it returns the needed buffer size (like most platforms and newer MSVC vesions)
Related
Commit: [r7662]
I can now run speed-dreams-2 with 700 character long file names on linux without crashing or getting errors by removing a lot of the snprintfs. There are still many remaining but it's a start.
Hi
since this change I have 23 warnings on GCC, I had 13 before, normal ?
Are you using gcc on windows?
That's strange as this change is inside of
Should not affect GCC
GCC 10 64bits on Linux