GNOME Bugzilla – Bug 756382
snprintf used on Windows with VS2015 doesn't support %n
Last modified: 2015-10-15 15:44:02 UTC
As reported in https://mail.gnome.org/archives/gtk-devel-list/2015-October/msg00007.html the vasnprintf() function from gnulib uses the CRT's snprintf function with a %n flag, which causes the program to abort when using the VS 2015 CRT. Earlier versions of VS CRT did not have snprintf at all so did not have a problem. VS2015 does provide it but aborts the program because it considers %n an invalid flag. This was fixed in upstream gnulib by not appending %n to the flags under Windows even if snprintf is available - http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blobdiff;f=lib/vasnprintf.c;h=968835a1e7d5a704ae5a330ecda091b78851f73c;hp=8377d31eb591687186815e1de5cf973aac465120;hb=ba739e5686c1488280341451c3eb01a8f7b0aaa1;hpb=8c1ff97529b4f5b6462f1be302cc526ccb1960a1 As suggested by LRN in the ML thread, glib could fix this by updating its copy of gnulib. I've worked around it for now by porting just that one line and it seems to be fine.
Created attachment 313104 [details] [review] gnulib: %n is not supported on old glibc or on native win32 On old glibc even if %n is supported we may get a crash, so it is better to skip it while on native win32 %n will abort the program. These changes were took from upstream.
So basically here I just took the code from upstream. I think it is fine but I have no way to test it on linux. I guess we have to wait for Matthias for a review on this.
Hi Nacho, (In reply to Ignacio Casal Quinteiro (nacho) from comment #2) > So basically here I just took the code from upstream. I think it is fine but > I have no way to test it on linux. I guess we have to wait for Matthias for > a review on this. I'd agree with you. With blessings, thank you!
I went ahead and I updated gnulib, It can be found in wip/nacho/gnulib.
Hi Nacho, I think it is fine to merge the gnulib updates in wip/nacho/gnulib into master (and glib-2-46 as well). I have tested it by building on Visual Studio 2008~2015 and ran the tests in glib\tests, which the related items passed (in particular the test-printf test program--it also ensures that the snprintf() implementation for Visual Studio 2015 is indeed good enough). With blessings, thank you!
Review of attachment 313104 [details] [review]: Please put this on glib-2-46. Meanwhile, improve the commit message on the branch to mention that it has been tested on Windows and also mention the changes you introduced vs. the upstream version. Attach that here and I will probably ACK it for master, but definitely not for backporting.
Comment on attachment 313104 [details] [review] gnulib: %n is not supported on old glibc or on native win32 Attachment 313104 [details] pushed as 94688bc - gnulib: %n is not supported on old glibc or on native win32
Created attachment 313361 [details] [review] Update gnulib It updates it to the version c5d07ce91a8ad51591154450442fa4376441fdfa As a difference with upstream we need to ensure: * Include "g-gnulib.h" so the methods get the gnulib namespace. * xsize.h uses G_MAXSIZE instead of SIZE_MAX and the methods are marked as static inline. * Some defines are named different from the ones in glib i.e HAVE_LONG_LONG_INT is HAVE_LONG_LONG All the unit tests pass properly with and without --enable-included-printf. It has also been tested on Windows.
Review of attachment 313361 [details] [review]: Thanks for being so thorough about this. Please commit, only to master.
Attachment 313361 [details] pushed as 212e423 - Update gnulib
glib/gnulib/printf-parse.c contains three instances of HAVE_LONG_LONG_INT
Created attachment 313384 [details] [review] gnulib: forgot some changes from HAVE_LONG_LONG_INT to HAVE_LONG_LONG
Comment on attachment 313384 [details] [review] gnulib: forgot some changes from HAVE_LONG_LONG_INT to HAVE_LONG_LONG Attachment 313384 [details] pushed as 0b84596 - gnulib: forgot some changes from HAVE_LONG_LONG_INT to HAVE_LONG_LONG
(In reply to Arnav Singh from comment #11) > glib/gnulib/printf-parse.c contains three instances of HAVE_LONG_LONG_INT Thanks, I recall changing it too, but at some point I had to overwrite and start from scratch so I guess I missed it on the second round. I pushed the new patch. Let me know if you still have issues.