[FFmpeg-devel] [PATCH 01/11] avformat/matroskadec: Simplify control flow of parsing laces

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Fri Aug 16 06:05:21 EEST 2019


Up until now, when an error happened in one of the inner loops in
matroska_parse_laces, a variable designated for the return value has
been set to an error value and break has been used to exit the
current loop/case. This was done so that the end of matroska_parse_laces
is reached, because said function allocates memory which is later used
and freed in the calling function and passed at the end of
matroska_parse_laces.
But there is actually no reason to pass it at the end: This commit sets
the address of the newly allocated memory as soon as it is known and
exits the subfunction as soon as an error is encountered.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavformat/matroskadec.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4e20f15792..25f26da074 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2999,7 +2999,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
                                 int *buf_size, int type,
                                 uint32_t **lace_buf, int *laces)
 {
-    int res = 0, n, size = *buf_size;
+    int n, size = *buf_size;
     uint8_t *data = *buf;
     uint32_t *lace_size;
 
@@ -3018,6 +3018,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
     data     += 1;
     size     -= 1;
     lace_size = av_malloc_array(*laces, sizeof(*lace_size));
+    *lace_buf = lace_size;
     if (!lace_size)
         return AVERROR(ENOMEM);
 
@@ -3026,13 +3027,12 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
     {
         uint8_t temp;
         uint32_t total = 0;
-        for (n = 0; res == 0 && n < *laces - 1; n++) {
+        for (n = 0; n < *laces - 1; n++) {
             lace_size[n] = 0;
 
             while (1) {
                 if (size <= total) {
-                    res = AVERROR_INVALIDDATA;
-                    break;
+                    return AVERROR_INVALIDDATA;
                 }
                 temp          = *data;
                 total        += temp;
@@ -3044,8 +3044,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
             }
         }
         if (size <= total) {
-            res = AVERROR_INVALIDDATA;
-            break;
+            return AVERROR_INVALIDDATA;
         }
 
         lace_size[n] = size - total;
@@ -3054,8 +3053,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
 
     case 0x2: /* fixed-size lacing */
         if (size % (*laces)) {
-            res = AVERROR_INVALIDDATA;
-            break;
+            return AVERROR_INVALIDDATA;
         }
         for (n = 0; n < *laces; n++)
             lace_size[n] = size / *laces;
@@ -3069,21 +3067,19 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
         if (n < 0 || num > INT_MAX) {
             av_log(matroska->ctx, AV_LOG_INFO,
                    "EBML block data error\n");
-            res = n<0 ? n : AVERROR_INVALIDDATA;
-            break;
+            return n<0 ? n : AVERROR_INVALIDDATA;
         }
         data += n;
         size -= n;
         total = lace_size[0] = num;
-        for (n = 1; res == 0 && n < *laces - 1; n++) {
+        for (n = 1; n < *laces - 1; n++) {
             int64_t snum;
             int r;
             r = matroska_ebmlnum_sint(matroska, data, size, &snum);
             if (r < 0 || lace_size[n - 1] + snum > (uint64_t)INT_MAX) {
                 av_log(matroska->ctx, AV_LOG_INFO,
                        "EBML block data error\n");
-                res = r<0 ? r : AVERROR_INVALIDDATA;
-                break;
+                return r<0 ? r : AVERROR_INVALIDDATA;
             }
             data        += r;
             size        -= r;
@@ -3091,8 +3087,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
             total       += lace_size[n];
         }
         if (size <= total) {
-            res = AVERROR_INVALIDDATA;
-            break;
+            return AVERROR_INVALIDDATA;
         }
         lace_size[*laces - 1] = size - total;
         break;
@@ -3100,10 +3095,9 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
     }
 
     *buf      = data;
-    *lace_buf = lace_size;
     *buf_size = size;
 
-    return res;
+    return 0;
 }
 
 static int matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
@@ -3537,7 +3531,7 @@ static int matroska_parse_block(MatroskaDemuxContext *matroska, AVBufferRef *buf
     int res = 0;
     AVStream *st;
     int16_t block_time;
-    uint32_t *lace_size = NULL;
+    uint32_t *lace_size;
     int n, flags, laces = 0;
     uint64_t num;
     int trust_default_duration = 1;
-- 
2.21.0



More information about the ffmpeg-devel mailing list