[3/6] error: Try using FormatMessageA for formatting error codes on windows

Message ID 1340112614-58001-1-git-send-email-martin@martin.st
State Deferred
Headers show

Commit Message

Martin Storsjö June 19, 2012, 1:30 p.m.
This provides more verbose error messages than the strerror
fallback in cmdutils.c (which might not necessarily be a good thing),
but it also is able to handle network error codes.
---
 libavutil/error.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Martin Storsjö June 21, 2012, 7 p.m. | #1
On Tue, 19 Jun 2012, Martin Storsjö wrote:

> This provides more verbose error messages than the strerror
> fallback in cmdutils.c (which might not necessarily be a good thing),
> but it also is able to handle network error codes.
> ---
> libavutil/error.c |   13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/error.c b/libavutil/error.c
> index 21b6876..0e7a26d 100644
> --- a/libavutil/error.c
> +++ b/libavutil/error.c
> @@ -18,6 +18,9 @@
>
> #include "avutil.h"
> #include "avstring.h"
> +#ifdef _WIN32
> +#include <windows.h>
> +#endif
>
> int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
> {
> @@ -45,7 +48,15 @@ int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
>     if (errstr) {
>         av_strlcpy(errbuf, errstr, errbuf_size);
>     } else {
> -#if HAVE_STRERROR_R
> +#ifdef _WIN32
> +        if (FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM |
> +                           FORMAT_MESSAGE_IGNORE_INSERTS |
> +                           FORMAT_MESSAGE_MAX_WIDTH_MASK,
> +                           NULL, AVUNERROR(errnum),
> +                           MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), errbuf,
> +                           errbuf_size, NULL) <= 0)
> +            ret = -1;
> +#elif HAVE_STRERROR_R
>         ret = strerror_r(AVUNERROR(errnum), errbuf, errbuf_size);
> #else
>         ret = -1;
> -- 
> 1.7.9.4

Ping, any opinions on this? Or should we use this one only for error codes 
in the winsock range, and use the plain strerror() as fallback? (I'm not 
sure if the msvcrt strerror() is threadsafe or not.) The error strings 
returned by this function is a bit more chatty than what one normally 
expect from strerror, but that one doesn't handle network errors on 
windows.

// Martin
Martin Storsjö June 25, 2012, 8:51 a.m. | #2
On Thu, 21 Jun 2012, Martin Storsjö wrote:

> On Tue, 19 Jun 2012, Martin Storsjö wrote:
>
>> This provides more verbose error messages than the strerror
>> fallback in cmdutils.c (which might not necessarily be a good thing),
>> but it also is able to handle network error codes.
>> ---
>> libavutil/error.c |   13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavutil/error.c b/libavutil/error.c
>> index 21b6876..0e7a26d 100644
>> --- a/libavutil/error.c
>> +++ b/libavutil/error.c
>> @@ -18,6 +18,9 @@
>> 
>> #include "avutil.h"
>> #include "avstring.h"
>> +#ifdef _WIN32
>> +#include <windows.h>
>> +#endif
>> 
>> int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
>> {
>> @@ -45,7 +48,15 @@ int av_strerror(int errnum, char *errbuf, size_t 
>> errbuf_size)
>>     if (errstr) {
>>         av_strlcpy(errbuf, errstr, errbuf_size);
>>     } else {
>> -#if HAVE_STRERROR_R
>> +#ifdef _WIN32
>> +        if (FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM |
>> +                           FORMAT_MESSAGE_IGNORE_INSERTS |
>> +                           FORMAT_MESSAGE_MAX_WIDTH_MASK,
>> +                           NULL, AVUNERROR(errnum),
>> +                           MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), 
>> errbuf,
>> +                           errbuf_size, NULL) <= 0)
>> +            ret = -1;
>> +#elif HAVE_STRERROR_R
>>         ret = strerror_r(AVUNERROR(errnum), errbuf, errbuf_size);
>> #else
>>         ret = -1;
>> -- 
>> 1.7.9.4
>
> Ping, any opinions on this? Or should we use this one only for error codes in 
> the winsock range, and use the plain strerror() as fallback? (I'm not sure if 
> the msvcrt strerror() is threadsafe or not.) The error strings returned by 
> this function is a bit more chatty than what one normally expect from 
> strerror, but that one doesn't handle network errors on windows.

Not only are the messages more chatty ("The system cannot find the file 
specified." compared to "No such file or directory"), but it actually uses 
error codes from a different range (winerror.h, which overlaps with 
errno.h and have error codes defined to totally different things), so 
instead of "Resource temporarily unavailable" for EAGAIN, one would get 
"An attempt was made to load a program with an incorrect format.".

So either this would only be used for error codes in the winsock range, or 
not be used at all.

Patch dropped for now at least.

// Martin

Patch

diff --git a/libavutil/error.c b/libavutil/error.c
index 21b6876..0e7a26d 100644
--- a/libavutil/error.c
+++ b/libavutil/error.c
@@ -18,6 +18,9 @@ 
 
 #include "avutil.h"
 #include "avstring.h"
+#ifdef _WIN32
+#include <windows.h>
+#endif
 
 int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
 {
@@ -45,7 +48,15 @@  int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
     if (errstr) {
         av_strlcpy(errbuf, errstr, errbuf_size);
     } else {
-#if HAVE_STRERROR_R
+#ifdef _WIN32
+        if (FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM |
+                           FORMAT_MESSAGE_IGNORE_INSERTS |
+                           FORMAT_MESSAGE_MAX_WIDTH_MASK,
+                           NULL, AVUNERROR(errnum),
+                           MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), errbuf,
+                           errbuf_size, NULL) <= 0)
+            ret = -1;
+#elif HAVE_STRERROR_R
         ret = strerror_r(AVUNERROR(errnum), errbuf, errbuf_size);
 #else
         ret = -1;