[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patch] Use inttypes macros for size_t format string


On 16.01.20 11:54, Andreas Schneider wrote:

> On Thursday, 16 January 2020 11:51:48 CET Andreas Schneider wrote:
>> On Wednesday, 15 January 2020 17:46:30 CET g4-lisz@xxxxxxxxxxxx wrote:
>>> On 15.01.20 17:32, Andreas Schneider wrote:
>>>> On Wednesday, 15 January 2020 14:35:39 CET Jakub Jelen wrote:
>>>>> On Wed, 2020-01-15 at 13:01 +0100, g4-lisz@xxxxxxxxxxxx wrote:
>>>>>> Hi there, here's a patch for fixing a printf format string issue when
>>>>>> compiling with MinGW (and possibly other "architectures"...).
>>>>>>
>>>>>> A big thanks to Zdenek OGAR Skalak for the hint!
>>>>> Looks good to me. Grepping through the rest of the code shows that
>>>>> there will most probably be more issues like this:
>>>>>
>>>>> $ git grep "PRIdS" | wc -l
>>>>> 10
>>>>> $ git grep "%zu" | wc -l
>>>>> 27
>>>>>
>>>>> Could you check also the other cases to make sure we can address it in
>>>>> the whole codebase?
>>>>>
>>>>> Andreas, what do you suggest to make sure we do not introduce new
>>>>> issues like this?
>>>> %zu is ANSI C99
>>>>
>>>> https://en.cppreference.com/w/c/io/fprintf
>>>>
>>>> That's more than 20 years old now. We had PRIdS because Visual Studio
>>>> didn't have %zu support for a long time. So we should replace PRIdS with
>>>> %zu ;-)
>>>>
>>>>
>>>> Till, I'm sorry but you hit a compiler bug, open a bug against MinGW.
>>> It's not really a compiler bug. See here:
>>> https://stackoverflow.com/questions/44382862/how-to-printf-a-size-t-withou
>>> t-> warning-in-mingw-w64-gcc-7-1
>> According to that you need to compile with:
>>
>> cmake -DCMAKE_C_FLAGS="-D__USE_MINGW_ANSI_STDIO=1" ..
> Alternative seems to be:
>
> -Dsnprintf=__mingw_snprintf -Dvsnprintf=__mingw_vsnprintf ...

Yes i was a bit overhasty with this patch...

I also found that _POSIX_SOURCE does the trick. This would be a bit more
generic.

But I realized that the binaries get bigger like that, probably because
they link in more of their own implementations. Not sure, if this is a
good thing...

-Dsnprintf=__mingw_snprintf -Dvsnprintf=__mingw_vsnprintf looks like a
good compromise to me. I will also test this.

Thanks,
Till



Follow-Ups:
Re: [patch] Use inttypes macros for size_t format stringAndreas Schneider <asn@xxxxxxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org