diff mbox

[FFmpeg-devel,4/5] avformat/avc: free buffer in ff_isom_write_avcc on failure

Message ID 20171129014303.3716-4-jamrial@gmail.com
State Accepted
Commit d5af8afbe4698273b2ef9b57487489b40f7888b1
Headers show

Commit Message

James Almer Nov. 29, 2017, 1:43 a.m. UTC 
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/avc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Michael Niedermayer Nov. 30, 2017, 1:12 a.m. UTC | #1 
On Tue, Nov 28, 2017 at 10:43:02PM -0300, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/avc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/avc.c b/libavformat/avc.c
> index 7542db684b..6ef6e08778 100644
> --- a/libavformat/avc.c
> +++ b/libavformat/avc.c
> @@ -145,8 +145,10 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>          buf += size;
>      }
>  
> -    if (!sps || !pps || sps_size < 4 || sps_size > UINT16_MAX || pps_size > UINT16_MAX)
> -        return AVERROR_INVALIDDATA;
> +    if (!sps || !pps || sps_size < 4 || sps_size > UINT16_MAX || pps_size > UINT16_MAX) {
> +        ret = AVERROR_INVALIDDATA;
> +        goto fail;
> +    }
>  
>      avio_w8(pb, 1); /* version */
>      avio_w8(pb, sps[1]); /* profile */
> @@ -160,9 +162,11 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>      avio_w8(pb, 1); /* number of pps */
>      avio_wb16(pb, pps_size);
>      avio_write(pb, pps, pps_size);
> +
> +fail:
>      av_free(start);

LGTM, but please set start = NULL at th top so "goto fail" is safe
at any point not just the current points

[...]
James Almer Nov. 30, 2017, 3:38 a.m. UTC | #2 
On 11/29/2017 10:12 PM, Michael Niedermayer wrote:
> On Tue, Nov 28, 2017 at 10:43:02PM -0300, James Almer wrote:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavformat/avc.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/avc.c b/libavformat/avc.c
>> index 7542db684b..6ef6e08778 100644
>> --- a/libavformat/avc.c
>> +++ b/libavformat/avc.c
>> @@ -145,8 +145,10 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>>          buf += size;
>>      }
>>  
>> -    if (!sps || !pps || sps_size < 4 || sps_size > UINT16_MAX || pps_size > UINT16_MAX)
>> -        return AVERROR_INVALIDDATA;
>> +    if (!sps || !pps || sps_size < 4 || sps_size > UINT16_MAX || pps_size > UINT16_MAX) {
>> +        ret = AVERROR_INVALIDDATA;
>> +        goto fail;
>> +    }
>>  
>>      avio_w8(pb, 1); /* version */
>>      avio_w8(pb, sps[1]); /* profile */
>> @@ -160,9 +162,11 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
>>      avio_w8(pb, 1); /* number of pps */
>>      avio_wb16(pb, pps_size);
>>      avio_write(pb, pps, pps_size);
>> +
>> +fail:
>>      av_free(start);
> 
> LGTM, but please set start = NULL at th top so "goto fail" is safe
> at any point not just the current points

Done.
diff mbox

Patch

diff --git a/libavformat/avc.c b/libavformat/avc.c
index 7542db684b..6ef6e08778 100644
--- a/libavformat/avc.c
+++ b/libavformat/avc.c
@@ -145,8 +145,10 @@  int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
         buf += size;
     }
 
-    if (!sps || !pps || sps_size < 4 || sps_size > UINT16_MAX || pps_size > UINT16_MAX)
-        return AVERROR_INVALIDDATA;
+    if (!sps || !pps || sps_size < 4 || sps_size > UINT16_MAX || pps_size > UINT16_MAX) {
+        ret = AVERROR_INVALIDDATA;
+        goto fail;
+    }
 
     avio_w8(pb, 1); /* version */
     avio_w8(pb, sps[1]); /* profile */
@@ -160,9 +162,11 @@  int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len)
     avio_w8(pb, 1); /* number of pps */
     avio_wb16(pb, pps_size);
     avio_write(pb, pps, pps_size);
+
+fail:
     av_free(start);
 
-    return 0;
+    return ret;
 }
 
 int ff_avc_write_annexb_extradata(const uint8_t *in, uint8_t **buf, int *size)