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
[Mingw-w64-public] [PATCH 04/18] winpthreads: Format error string with snprintf
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(-)
diff --git a/mingw-w64-libraries/winpthreads/src/thread.c
b/mingw-w64-libraries/winpthreads/src/thread.c
index d5c2acf1f..f48923e29 100644
--- a/mingw-w64-libraries/winpthreads/src/thread.c
+++ b/mingw-w64-libraries/winpthreads/src/thread.c
@@ -411,24 +411,11 @@ replace_spin_keys (pthread_spinlock_t *old,
pthread_spinlock_t new)
if (EPERM == pthread_spin_destroy (old))
{
-#define THREADERR "Error cleaning up spin_keys for thread "
-#define THREADERR_LEN ((sizeof (THREADERR) / sizeof (*THREADERR)) - 1)
-#define THREADID_LEN THREADERR_LEN + 66 + 1 + 1
- int i;
- char thread_id[THREADID_LEN] = THREADERR;
- _ultoa ((unsigned long) GetCurrentThreadId (),
_id[THREADERR_LEN], 10);
- for (i = THREADERR_LEN; thread_id[i] != '\0' && i < THREADID_LEN - 1;
i++)
-{
-}
- if (i < THREADID_LEN - 1)
-{
- thread_id[i] = '\n';
- thread_id[i + 1] = '\0';
-}
+#define THREADERR "Error cleaning up spin_keys for thread %lu.\n"
+ char threaderr[sizeof(THREADERR) + 8] = { 0 };
+ snprintf(threaderr, sizeof(threaderr), THREADERR, GetCurrentThreadId());
#undef THREADERR
-#undef THREADERR_LEN
-#undef THREADID_LEN
- OutputDebugStringA (thread_id);
+ OutputDebugStringA (threaderr);
abort ();
}
--
2.43.0
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
