Janne Hyv?rinen wrote:> Some comments for patch #1, I chose the non-secure versions because they > are faster and produce smaller binary. The functions where these > printings are performed can't in my opinion ever exceed the safety > margin of 32 KB. They print short help and error texts and occasionally > filename, which with APIs is restricted to 260 characters. And you can't > feed it longer faulty names either because maximum command line length > is much shorter than 32 KB.I still like this patch. The secure versions might be slightly slower, but they are nowhere near the speed critical parts of the FLAC code. They also *document* the fact that they are safe so you really only need to look where they are called rather than thinking about maximum command line lengths and maximum file name lengths. Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
Erik de Castro Lopo wrote:> Janne Hyv?rinen wrote: > >> Some comments for patch #1, I chose the non-secure versions because they >> are faster and produce smaller binary. The functions where these >> printings are performed can't in my opinion ever exceed the safety >> margin of 32 KB. They print short help and error texts and occasionally >> filename, which with APIs is restricted to 260 characters. And you can't >> feed it longer faulty names either because maximum command line length >> is much shorter than 32 KB. > > I still like this patch. The secure versions might be slightly slower, > but they are nowhere near the speed critical parts of the FLAC code. > They also *document* the fact that they are safe so you really only > need to look where they are called rather than thinking about maximum > command line lengths and maximum file name lengths.Oops. It seems that vsnprintf_s isn't always available on MinGW platform: MinGW declares this function only if MINGW_HAS_SECURE_API macro is defined. That's because WinXP version of msvcrt.dll doesn't contain secure functions like vsnprintf_s. Maybe it's better to revert vsnprintf_s to vsprintf or to use vnsprintf? (Of course it's also possible to write something like #if defined _MSC_VER || defined MINGW_HAS_SECURE_API ret = vsnprintf_s(buf, bufsize, _TRUNCATE, format, arglist); #else ret = vsnprintf(buf, bufsize, format, arglist); if (ret < 0) buf[bufsize-1] = '\0'; #endif but it probably just adds unnecessary complexity)
Erik de Castro Lopo
2014-Sep-18 09:53 UTC
[flac-dev] patch for win_utf8_io.c: vsnprintf_s vs. MinGW
lvqcl wrote:> Oops. It seems that vsnprintf_s isn't always available on MinGW platform: > MinGW declares this function only if MINGW_HAS_SECURE_API macro is defined. > That's because WinXP version of msvcrt.dll doesn't contain secure functions > like vsnprintf_s.I thought Micorsoft had recently stopped supporting WinXP. If Micorsoft has stopped supporting it I see no reason for FLAC to support it. Cheers, Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
Erik de Castro Lopo
2014-Sep-18 12:14 UTC
[flac-dev] patch for win_utf8_io.c: vsnprintf_s vs. MinGW
lvqcl wrote:> Oops. It seems that vsnprintf_s isn't always available on MinGW platform: > MinGW declares this function only if MINGW_HAS_SECURE_API macro is defined. > That's because WinXP version of msvcrt.dll doesn't contain secure functions > like vsnprintf_s. > > Maybe it's better to revert vsnprintf_s to vsprintf or to use vnsprintf?Ok, we need to drop vsnprintf_s to support WinXP. I'd prefer vsnprintf over vsprintf but have no way of testing any of these options. Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/