On 16.11.2017 14:59, Martin Storsjö wrote:
> On Thu, 16 Nov 2017, Jacek Caban wrote:
>
>> Hi Martin,
>>
>> The patch looks generally good to me. I'm fine with committing it.
>>
>> As a side note, I expect this patch to also fix winpthread (built with
>> toolchain defaulting default msvcrt.dll) linking problems that I saw
>> when
>> tried your patches.
>>
>> On 15.11.2017 23:11, Martin Storsjö wrote:
>>
>> This allows build scripts that check for functions without including
>> stdio.h (but just checking if the linker finds a symbol with the
>> given name) to detect the presence of these functions.
>>
>> The asprintf and vasprintf functions get included from libmingwex,
>> and they are implemented in terms of _vsnprintf, so that function
>> must be implemented.
>>
>> Signed-off-by: Martin Storsjö <mar...@martin.st>
>> ---
>> mingw-w64-crt/Makefile.am | 11 +++-
>> mingw-w64-crt/crt/ucrtbase_compat.c | 26 +---------
>> mingw-w64-crt/stdio/ucrt__vsnprintf.c | 14 +++++
>> mingw-w64-crt/stdio/ucrt_fprintf.c | 19 +++++++
>> mingw-w64-crt/stdio/ucrt_printf.c | 19 +++++++
>> mingw-w64-crt/stdio/ucrt_snprintf.c | 19 +++++++
>> mingw-w64-crt/stdio/ucrt_sprintf.c | 19 +++++++
>> mingw-w64-crt/stdio/ucrt_vfprintf.c | 14 +++++
>> mingw-w64-crt/stdio/ucrt_vprintf.c | 14 +++++
>> mingw-w64-crt/stdio/ucrt_vsnprintf.c | 14 +++++
>> mingw-w64-crt/stdio/ucrt_vsprintf.c | 14 +++++
>> mingw-w64-headers/crt/stdio.h | 97
>> ++++----------------------------
>> ---
>> 12 files changed, 168 insertions(+), 112 deletions(-)
>> create mode 100644 mingw-w64-crt/stdio/ucrt__vsnprintf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_fprintf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_printf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_snprintf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_sprintf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_vfprintf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_vprintf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_vsnprintf.c
>> create mode 100644 mingw-w64-crt/stdio/ucrt_vsprintf.c
>>
>>
>> It looks like we'll have quite a few file for ucrt. It may make sense to
>> have ucrt/ dir at some point.
>>
>> diff --git a/mingw-w64-crt/stdio/ucrt_printf.c
>> b/mingw-w64-crt/stdio/ucrt_pr
>> intf.c
>> new file mode 100644
>> index 0000000..d4a6e82
>> --- /dev/null
>> +++ b/mingw-w64-crt/stdio/ucrt_printf.c
>> @@ -0,0 +1,19 @@
>> +/**
>> + * This file has no copyright assigned and is placed in the Public
>> Domain.
>> + * This file is part of the mingw-w64 runtime package.
>> + * No warranty is given; refer to the file DISCLAIMER.PD within this
>> packag
>> e.
>> + */
>> +
>> +#undef __MSVCRT_VERSION__
>> +#define __MSVCRT_VERSION__ 0x1400
>> +#include <stdio.h>
>> +
>> +int __cdecl printf(const char * __restrict__ _Format,...)
>> +{
>> + __builtin_va_list ap;
>> + int ret;
>> + __builtin_va_start(ap, _Format);
>> + ret = __stdio_common_vfprintf(0, stdout, _Format, NULL, ap);
>> + __builtin_va_end(ap);
>> + return ret;
>> +}
>>
>>
>> The first argument with compatibility flags seems inconsistent between
>> implementations. Is there a reason for that? It's fine for the patch,
>> you
>> preserve headers behaviour, but it may be nice to revisit that at some
>> point.
>
> Yes, they're subtly different - almost (but not quite) what they were
> in their inline form in the headers.
>
>
> In general we want these functions to behave according to standards,
> but e.g. for nonstandard functions like _snprintf, I think it's better
> to preserve the behaviour of how this function used to behave (i.e not
> writing any null termination) than to make it identical to normal
> snprintf.
That doesn't sound right. According to [1] those underscored functions
behave the same as not underscored on MSVC. They are also documented to
be preferred, according to Microsoft. That said, a code written against
ucrtbase according to documentation will use underscored versions and
expect then to work the same as non-underscored. Such applications would
consider (correctly, I believe) mingw-w64 behaviour as broken.
In general, I think, it goes down to not introducing incompatibilities
with MSVC unless we have a reason to.
>
>
> I did change _vsnprintf a little though, when making it non-inline. In
> the header form, I had _vsnprintf also behaving like _snprintf, in the
> legacy mode.
>
> When we make (v)asprintf non-inline, they end up calling the
> implementation of (v)asprintf from libmingwex, so there's no point in
> trying to provide a version in libucrtbase.a.
>
> The libmingwex implementation of (v)asprintf calls _vsnprintf. When
> testing things with wine (which I do a lot), this actually didn't work
> when I had the _vsnprintf implementation of use
> UCRTBASE_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION, I had to change it
> to UCRTBASE_PRINTF_STANDARD_SNPRINTF_BEHAVIOUR. With the real ucrtbase
> on real windows, UCRTBASE_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION
> works here, so it seems like it's a missed cornercase in the
> implementation of these flags in wine (where I only have myself to
> blame).
>
> I guess I should change it to consistently use
> UCRTBASE_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION in _vsnprintf to
> match _snprintf and fix wine instead.
Oh, please, don't ever try to work around Wine bugs like that. Wine
ucrtbase.dll is not yet mature, but I expect things will get better over
time. Right now there are probably tons of subtle differences there. I'm
sure it will be fixed in Wine at some point. For purpose of testing for
mingw-w64, I'd suggest to simply use native ucrtbase.dll in Wine. It
will prevent misinterpretations of results.
Thanks,
Jacek
[1] https://msdn.microsoft.com/en-us/library/sk54f3f5.aspx
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
