lvqcl wrote:> Why? We can use vsnprintf_s for MSVS, and vsnprintf for MinGW. > > MSVS version of vsnprintf_s is located inside (statically linked) msvcp???.lib > or (dynamically linked) msvcp???.dll. They are part of MSVS runtime, and compatible > with WinXP. So it is safe to use it in FLAC.Oh, ok. I missed this bit. I know so very little about Windows. However, if you compile flac on say Win7 using vsnprintf_s and then take the dynamically compiled binary to WinXP it will fail, right?> As you see, MSVC version of vsnprintf can return positive value > even when it didn't write '\0' at the end. So > if (rc < 0) { ... str [...] = 0; } > is not enough.Like I said, untested. It was the first thing that came into my head version of the code :-).> What about this version -- > > int flac_snprintf(char *str, size_t size, const char *fmt, ...) > { > va_list va; > int rc; > > va_start (va, fmt); > > #if defined _MSC_VER > rc = vsnprintf_s (str, size, _TRUNCATE, fmt, va); > if (rc < 0) > rc = size - 1; > #elif defined __MINGW32__ && (!defined __USE_MINGW_ANSI_STDIO || __USE_MINGW_ANSI_STDIO == 0) > rc = vsnprintf (str, size, fmt, va); > if (rc < 0 || rc == size) { > rc = size - 1; > str [rc] = '\0'; /* assert(size > 0) */ > } > #else > rc = vsnprintf (str, size, fmt, va); > #endif > va_end (va); > > return rc; > }I like that but ....> Or forget about __USE_MINGW_ANSI_STDIO: > > int flac_snprintf(char *str, size_t size, const char *fmt, ...) > { > va_list va; > int rc; > > va_start (va, fmt); > > #if defined _MSC_VER || defined __MINGW32__ > rc = vsnprintf (str, size, fmt, va); > if (rc < 0 || rc == size) { > rc = size - 1; > str [rc] = '\0'; /* assert(size > 0) */ > } > #else > rc = vsnprintf (str, size, fmt, va); > #endif > va_end (va); > > return rc; > }This one has the advantage of simplicity, there are only two things to test. Either of those two are fine. You choose and send a patch. I'll write a unit test for flac_snprintf to capture our assumptions. Cheers, Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
Erik de Castro Lopo wrote:>> MSVS version of vsnprintf_s is located inside (statically linked) msvcp???.lib >> or (dynamically linked) msvcp???.dll. They are part of MSVS runtime, and compatible >> with WinXP. So it is safe to use it in FLAC. > > Oh, ok. I missed this bit. I know so very little about Windows. > > However, if you compile flac on say Win7 using vsnprintf_s and then take > the dynamically compiled binary to WinXP it will fail, right?No, flac.exe compiled with MSVS 2013 (and dynamically linked with MSVC runtime) will simply require msvcr120.dll to run. On both OSes.> Either of those two are fine. You choose and send a patch. I'll write > a unit test for flac_snprintf to capture our assumptions.I would like to know opinions of other people... Especially because I'm not very familiar with MinGW. Anyone? P.S. There are 3 places where flac uses vsnprintf: 1) flac_snprintf inside src/share/grabbag/snprintf.c 2) local_snprintf inside src/libFLAC/metadata_iterators.c -- but it's a full copy of flac_snprintf 3) printf_utf8/fprintf_utf8/vfprintf_utf8 inside src/share/win_utf8_io/win_utf8_io.c
On 9/21/2014 06:58, lvqcl wrote:> Erik de Castro Lopo wrote: > >>> MSVS version of vsnprintf_s is located inside (statically linked) msvcp???.lib >>> or (dynamically linked) msvcp???.dll. They are part of MSVS runtime, and compatible >>> with WinXP. So it is safe to use it in FLAC. >> >> Oh, ok. I missed this bit. I know so very little about Windows. >> >> However, if you compile flac on say Win7 using vsnprintf_s and then take >> the dynamically compiled binary to WinXP it will fail, right? > > No, flac.exe compiled with MSVS 2013 (and dynamically linked with MSVC runtime) will > simply require msvcr120.dll to run. On both OSes. >You could just request mingw to include a vsnprintf_s implementation for XP and earlier, mingw-w64 has already done so. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: OpenPGP digital signature Url : http://lists.xiph.org/pipermail/flac-dev/attachments/20140921/c6578921/attachment.pgp
lvqcl wrote:> I would like to know opinions of other people... Especially because I'm not > very familiar with MinGW. Anyone?Is it safe to use __mingw_vsnprintf? If yes, then flac_snprintf can look like: int flac_snprintf(char *str, size_t size, const char *fmt, ...) { va_list va; int rc; va_start (va, fmt); #if defined _MSC_VER rc = vsnprintf_s (str, size, _TRUNCATE, fmt, va); if (rc < 0) rc = something_that_makes_sense; #elif defined __MINGW32__ rc = __mingw_vsnprintf (str, size, fmt, va); #else rc = vsnprintf (str, size, fmt, va); #endif va_end (va); return rc; }