commit 388889c2a7be7102e4cd26cf161d116ea3198ac0
author Joe Drago <jdrago@netflix.com> Tue May 04 13:03:31 2021 -0700
committer Joe Drago <jdrago@netflix.com> Tue May 04 13:03:31 2021 -0700
tree 6b108bf26c1cfe04721698c621dc48fec31ef6e8
parent ba4d67d4700c9fa89c48de693832590bb6dc6f8a

Simplify avifDiagnosticsPrintf(), remove redundant NULL diag checks in avifROStream functions

src/diag.c

diff --git a/src/diag.c b/src/diag.c
index 3b8c244..d9cb6ea 100644
--- a/src/diag.c
+++ b/src/diag.c
@@ -12,26 +12,20 @@
     memset(diag->error, 0, AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE);
 }
 
-static void avifDiagnosticsPrintv(avifDiagnostics * diag, const char * format, va_list args)
-{
-    if (*diag->error) {
-        // There is already a detailed error set.
-        return;
-    }
-
-    vsnprintf(diag->error, AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE, format, args);
-    diag->error[AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE - 1] = '\0';
-}
-
 void avifDiagnosticsPrintf(avifDiagnostics * diag, const char * format, ...)
 {
     if (!diag) {
         // It is possible this is NULL (e.g. calls to avifFileTypeIsCompatible())
         return;
     }
+    if (*diag->error) {
+        // There is already a detailed error set.
+        return;
+    }
 
     va_list args;
     va_start(args, format);
-    avifDiagnosticsPrintv(diag, format, args);
+    vsnprintf(diag->error, AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE, format, args);
+    diag->error[AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE - 1] = '\0';
     va_end(args);
 }

src/stream.c

diff --git a/src/stream.c b/src/stream.c
index 9b02e96..eb0eabb 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -53,9 +53,7 @@
 avifBool avifROStreamSkip(avifROStream * stream, size_t byteCount)
 {
     if (!avifROStreamHasBytesLeft(stream, byteCount)) {
-        if (stream->diag) {
-            avifDiagnosticsPrintf(stream->diag, "%s: Failed to skip %zu bytes, truncated data?", stream->diagContext, byteCount);
-        }
+        avifDiagnosticsPrintf(stream->diag, "%s: Failed to skip %zu bytes, truncated data?", stream->diagContext, byteCount);
         return AVIF_FALSE;
     }
     stream->offset += byteCount;
@@ -65,9 +63,7 @@
 avifBool avifROStreamRead(avifROStream * stream, uint8_t * data, size_t size)
 {
     if (!avifROStreamHasBytesLeft(stream, size)) {
-        if (stream->diag) {
-            avifDiagnosticsPrintf(stream->diag, "%s: Failed to read %zu bytes, truncated data?", stream->diagContext, size);
-        }
+        avifDiagnosticsPrintf(stream->diag, "%s: Failed to read %zu bytes, truncated data?", stream->diagContext, size);
         return AVIF_FALSE;
     }
 
@@ -99,9 +95,7 @@
         *v = tmp;
     } else {
         // Unsupported factor
-        if (stream->diag) {
-            avifDiagnosticsPrintf(stream->diag, "%s: Failed to read UX8 value; Unsupported UX8 factor [%" PRIu64 "]", stream->diagContext, factor);
-        }
+        avifDiagnosticsPrintf(stream->diag, "%s: Failed to read UX8 value; Unsupported UX8 factor [%" PRIu64 "]", stream->diagContext, factor);
         return AVIF_FALSE;
     }
     return AVIF_TRUE;
@@ -141,9 +135,7 @@
         }
     }
     if (!foundNullTerminator) {
-        if (stream->diag) {
-            avifDiagnosticsPrintf(stream->diag, "%s: Failed to find a NULL terminator when reading a string", stream->diagContext);
-        }
+        avifDiagnosticsPrintf(stream->diag, "%s: Failed to find a NULL terminator when reading a string", stream->diagContext);
         return AVIF_FALSE;
     }
 
@@ -181,9 +173,7 @@
 
     size_t bytesRead = stream->offset - startOffset;
     if ((size < bytesRead) || ((size - bytesRead) > SIZE_MAX)) {
-        if (stream->diag) {
-            avifDiagnosticsPrintf(stream->diag, "%s: Header size overflow check failure", stream->diagContext);
-        }
+        avifDiagnosticsPrintf(stream->diag, "%s: Header size overflow check failure", stream->diagContext);
         return AVIF_FALSE;
     }
     header->size = (size_t)(size - bytesRead);
@@ -193,7 +183,11 @@
 avifBool avifROStreamReadBoxHeader(avifROStream * stream, avifBoxHeader * header)
 {
     CHECK(avifROStreamReadBoxHeaderPartial(stream, header));
-    return (header->size <= avifROStreamRemainingBytes(stream));
+    if (header->size > avifROStreamRemainingBytes(stream)) {
+        avifDiagnosticsPrintf(stream->diag, "%s: Child box too large, possibly truncated data", stream->diagContext);
+        return AVIF_FALSE;
+    }
+    return AVIF_TRUE;
 }
 
 avifBool avifROStreamReadVersionAndFlags(avifROStream * stream, uint8_t * version, uint32_t * flags)
@@ -214,9 +208,7 @@
     uint8_t version;
     CHECK(avifROStreamReadVersionAndFlags(stream, &version, NULL));
     if (version != enforcedVersion) {
-        if (stream->diag) {
-            avifDiagnosticsPrintf(stream->diag, "%s: Expecting box version %u, got version %u", stream->diagContext, enforcedVersion, version);
-        }
+        avifDiagnosticsPrintf(stream->diag, "%s: Expecting box version %u, got version %u", stream->diagContext, enforcedVersion, version);
         return AVIF_FALSE;
     }
     return AVIF_TRUE;