Re: [Mingw-w64-public] [PATCH 04/18] winpthreads: Format error string with snprintf
Le mer. 29 nov. 2023 à 17:26, Martin Storsjö a écrit : > > On Wed, 29 Nov 2023, LIU Hao wrote: > > > 在 2023/11/29 18:38, Antonin Décimo 写道: > >> Previous code was too complex and hard to understand, and snprintf > >> fits the job nicely. > >> > >> Signed-off-by: Antonin Décimo > >> --- > >> mingw-w64-libraries/winpthreads/src/thread.c | 21 > >> > >> 1 file changed, 4 insertions(+), 17 deletions(-) > >> > > > >> + char threaderr[sizeof(THREADERR) + 8] = { 0 }; > >> + snprintf(threaderr, sizeof(threaderr), THREADERR, > >> GetCurrentThreadId()); > > > > > I think `sprintf()` should be called here, as the length of the output > > string has an upper limit. > > Even for bounded output sizes, it may make sense to avoid sprintf - many > environments warn on any use of it, even if the buffer can be guaranteed > to be large enough. Although I'm not sure if any of our tools warn about > it. > > > There wasn't `snprintf()` in MSVCRT.DLL from Windows XP. > > Actually, no msvcrt.dll on any system has got the snprintf function. > > In msvcrt builds (without our ansi stdio support enabled), when calling > snprintf, it calls our internal __ms_snprintf. > > This function is implemented with _vscprintf. This function exists in > msvcrt.dll since XP, but is absent on 2k and earlier. However > dd72602ea5ed0e612d9825ede518970ed810dbb7 added a fallback implementation, > making our snprintf safe to use for any target. > > // Martin Thanks for the explanations! Am I correct in understanding that this patch is fine as-is and won't break any currently supported targets? Note that VS2022 target support stops at Win7 and Server 2008 R2. I'm not interested in adding support for olders MSVC or Windows systems (using MSVC), but of course, I'd rather not break anything that's currently working. https://learn.microsoft.com/en-us/visualstudio/releases/2022/compatibility#developWindows -- Antonin ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH 04/18] winpthreads: Format error string with snprintf
On Wed, 29 Nov 2023, LIU Hao wrote: 在 2023/11/29 18:38, Antonin Décimo 写道: Previous code was too complex and hard to understand, and snprintf fits the job nicely. Signed-off-by: Antonin Décimo --- mingw-w64-libraries/winpthreads/src/thread.c | 21 1 file changed, 4 insertions(+), 17 deletions(-) + char threaderr[sizeof(THREADERR) + 8] = { 0 }; + snprintf(threaderr, sizeof(threaderr), THREADERR, GetCurrentThreadId()); I think `sprintf()` should be called here, as the length of the output string has an upper limit. Even for bounded output sizes, it may make sense to avoid sprintf - many environments warn on any use of it, even if the buffer can be guaranteed to be large enough. Although I'm not sure if any of our tools warn about it. There wasn't `snprintf()` in MSVCRT.DLL from Windows XP. Actually, no msvcrt.dll on any system has got the snprintf function. In msvcrt builds (without our ansi stdio support enabled), when calling snprintf, it calls our internal __ms_snprintf. This function is implemented with _vscprintf. This function exists in msvcrt.dll since XP, but is absent on 2k and earlier. However dd72602ea5ed0e612d9825ede518970ed810dbb7 added a fallback implementation, making our snprintf safe to use for any target. // Martin ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
Re: [Mingw-w64-public] [PATCH 04/18] winpthreads: Format error string with snprintf
在 2023/11/29 18:38, Antonin Décimo 写道: Previous code was too complex and hard to understand, and snprintf fits the job nicely. Signed-off-by: Antonin Décimo --- mingw-w64-libraries/winpthreads/src/thread.c | 21 1 file changed, 4 insertions(+), 17 deletions(-) + char threaderr[sizeof(THREADERR) + 8] = { 0 }; + snprintf(threaderr, sizeof(threaderr), THREADERR, GetCurrentThreadId()); There wasn't `snprintf()` in MSVCRT.DLL from Windows XP. I think `sprintf()` should be called here, as the length of the output string has an upper limit. -- Best regards, LIU Hao OpenPGP_signature.asc Description: OpenPGP digital signature ___ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public