[FFmpeg-devel] [PATCH] mingw: Use avprive_vsnprintf in bprint

[jamal]

On 20/09/12 5:23 PM, Derek Buitenhuis wrote:
> On 20/09/2012 3:53 PM, jamal wrote:
>> I think HAVE_WINDOWS_H should work for that.
> 
> We already have something like HAVE_MSVCRT or so. There's no reason for
> an extra check. Heck, we may as well just use _WIN32.

HAVE_WINDOWS_H already exists, that's why i mentioned it.

>> Nonetheless, Mingw32's vsnprintf works fine. The buggy one is Mingw64's (both 32 and 64 bits) before rev 4663 as you mentioned, so we should make sure to use avpriv_vsnprintf only with Mingw64 v1.0 and v2.0.
> 
> I'd rather just use avpriv_vsnprint everywhere instead of extra version checks.
> Anyone who still uses pre-xp OSes is not fit to be using a computer.
> 
>> This is important because Mingw32 is the only remaining toolchain that compiles for Win98/NT4 and Win2K, so forcing the usage of avpriv_vsnprintf with it would mean the end of ffmpeg's support for those versions.
> 
> See above.

I personally don't agree with dropping support for Win98/NT4/2K when there's no justified reason for doing so.
If we were trying to add some new feature or fix that would bring improvements to current Windows versions and the only way to achieve that is by dropping support for old versions, then I'd agree since it would be justified. But this you suggest is dropping support simply to avoid doing some extra checks.

>> And regarding the prototypes, i still think that a new libavutil/os_support.h header for this would be a good idea.
> 
> I am certain this will be frowned upon... see all the bikeshedding about where the implementation was
> placed in the first place...

What about adding new functions av_vsnprintf and av_snprintf to avstring.h/c, and replacing every instance of vsnprintf and snprintf with them?
With MSVC and MingW64 < v3.0 they would use the code that's currently on snprintf.c, and with everything else they would simply call the corresponding system function. Check for example the attached patch.

If neither that nor libavutil/os_support.h are viable options, then we could just force the inclusion of some new header in CFLAGS, which is what Bultje originally intended and what's being done with MSVC for other stuff.
It's hackish and i personally don't like it, but it's acceptable.

Regards.
-------------- next part --------------
diff --git a/libavutil/avstring.c b/libavutil/avstring.c
index 8b258a4..416309e 100644
--- a/libavutil/avstring.c
+++ b/libavutil/avstring.c
@@ -118,6 +118,49 @@ end:
     return p;
 }
 
+int av_snprintf(char *s, size_t n, const char *fmt, ...)
+{
+    va_list ap;
+    int ret;
+
+    va_start(ap, fmt);
+    ret = av_vsnprintf(s, n, fmt, ap);
+    va_end(ap);
+
+    return ret;
+}
+
+int av_vsnprintf(char *s, size_t n, const char *fmt,
+                     va_list ap)
+{
+#if defined(__MSVC__) || defined(__MINGW64_VERSION_MAJOR) && (__MINGW64_VERSION_MAJOR < 3)
+    int ret;
+    va_list ap_copy;
+
+    if (n == 0)
+        return _vscprintf(fmt, ap);
+    else if (n > INT_MAX)
+        return AVERROR(EFBIG);
+
+    /* we use n - 1 here because if the buffer is not big enough, the MS
+     * runtime libraries don't add a terminating zero at the end. MSDN
+     * recommends to provide _snprintf/_vsnprintf() a buffer size that
+     * is one less than the actual buffer, and zero it before calling
+     * _snprintf/_vsnprintf() to workaround this problem.
+     * See http://msdn.microsoft.com/en-us/library/1kt27hek(v=vs.80).aspx */
+    memset(s, 0, n);
+    va_copy(ap_copy, ap);
+    ret = _vsnprintf(s, n - 1, fmt, ap_copy);
+    va_end(ap_copy);
+    if (ret == -1)
+        ret = _vscprintf(fmt, ap);
+
+    return ret;
+#else
+    return vsnprintf(s, n, fmt, ap);
+#endif
+}
+
 char *av_d2str(double d)
 {
     char *str= av_malloc(16);
diff --git a/libavutil/avstring.h b/libavutil/avstring.h
index f73d6e7..5dc7ead 100644
--- a/libavutil/avstring.h
+++ b/libavutil/avstring.h
@@ -125,6 +125,9 @@ size_t av_strlcatf(char *dst, size_t size, const char *fmt, ...) av_printf_forma
  */
 char *av_asprintf(const char *fmt, ...) av_printf_format(1, 2);
 
+int av_snprintf(char *s, size_t n, const char *fmt, ...) av_printf_format(3, 4);
+int av_vsnprintf(char *s, size_t n, const char *fmt, va_list ap);
+
 /**
  * Convert a number to a av_malloced string.
  */