diff mbox series

[FFmpeg-devel,RFC,v2] avutil/error: Provide better feedback about unknown error codes

Message ID 20240716111543.2437488-2-ffmpeg-devel@pileofstuff.org
State New
Headers show
Series [FFmpeg-devel,RFC,v2] avutil/error: Provide better feedback about unknown error codes | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andrew Sayers July 16, 2024, 11:13 a.m. UTC 
AVERROR messages should always be less than zero,
and are usually based on three or four ASCII characters.

For error codes that aren't explicitly handled by error.c (e.g. FFERROR_REDO),
print the ASCII code so the user has a little more information.

If a non-negative number somehow gets passed to this function,
print a message saying this shouldn't happen.
---
 libavutil/error.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer July 17, 2024, 9:06 p.m. UTC | #1 
On Tue, Jul 16, 2024 at 12:13:10PM +0100, Andrew Sayers wrote:
> AVERROR messages should always be less than zero,
> and are usually based on three or four ASCII characters.
> 
> For error codes that aren't explicitly handled by error.c (e.g. FFERROR_REDO),
> print the ASCII code so the user has a little more information.
> 
> If a non-negative number somehow gets passed to this function,
> print a message saying this shouldn't happen.
[...]

> +    } else if (errnum >= 0) {
> +        snprintf(errbuf, errbuf_size, "Impossible: non-negative error number %d occurred, please report this bug", errnum);
>      } else {
>  #if HAVE_STRERROR_R
>          ret = AVERROR(strerror_r(AVUNERROR(errnum), errbuf, errbuf_size));
> @@ -126,7 +160,7 @@ int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
>          ret = -1;
>  #endif
>          if (ret < 0)
> -            snprintf(errbuf, errbuf_size, "Error number %d occurred", errnum);
> +            snprintf(errbuf, errbuf_size, "Error number -0x%X occurred, please report this bug", -errnum);
>      }

I think this (asking for a report and pointing out to the user that
this isnt supposed to happen), is a good idea

thx

[...]
diff mbox series

Patch

diff --git a/libavutil/error.c b/libavutil/error.c
index 90bab7b9d3..9706578be6 100644
--- a/libavutil/error.c
+++ b/libavutil/error.c
@@ -119,6 +119,40 @@  int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
     }
     if (entry) {
         av_strlcpy(errbuf, entry->str, errbuf_size);
+    } else if (
+        -errnum <= 0xFFFFFFFF
+        && ((-errnum >>  0) & 0xFF) >= 0x20 && ((-errnum >>  0) & 0xFF) <= 0x7F
+        && ((-errnum >>  8) & 0xFF) >= 0x20 && ((-errnum >>  8) & 0xFF) <= 0x7F
+        && ((-errnum >> 16) & 0xFF) >= 0x20 && ((-errnum >> 16) & 0xFF) <= 0x7F
+        && (
+           (((-errnum >> 24) & 0xFF) >= 0x20 && ((-errnum >> 24) & 0xFF) <= 0x7F)
+           || !((-errnum >> 24) & 0xFF)
+        )
+    ) {
+        if ((-errnum >> 24) & 0xFF) {
+            snprintf(
+                errbuf,
+                errbuf_size,
+                "Error number -0x%x (\"%c%c%c%c\") occurred, please report this bug",
+                -errnum,
+                (-errnum >>  0) & 0xFF,
+                (-errnum >>  8) & 0xFF,
+                (-errnum >> 16) & 0xFF,
+                (-errnum >> 24) & 0xFF
+            );
+        } else {
+            snprintf(
+                errbuf,
+                errbuf_size,
+                "Error number -0x%x (\"%c%c%c\") occurred, please report this bug",
+                -errnum,
+                (-errnum >>  0) & 0xFF,
+                (-errnum >>  8) & 0xFF,
+                (-errnum >> 16) & 0xFF
+            );
+        }
+    } else if (errnum >= 0) {
+        snprintf(errbuf, errbuf_size, "Impossible: non-negative error number %d occurred, please report this bug", errnum);
     } else {
 #if HAVE_STRERROR_R
         ret = AVERROR(strerror_r(AVUNERROR(errnum), errbuf, errbuf_size));
@@ -126,7 +160,7 @@  int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
         ret = -1;
 #endif
         if (ret < 0)
-            snprintf(errbuf, errbuf_size, "Error number %d occurred", errnum);
+            snprintf(errbuf, errbuf_size, "Error number -0x%X occurred, please report this bug", -errnum);
     }
 
     return ret;