diff mbox

[FFmpeg-devel,06/11] cbs, cbs_mpeg2, cbs_jpeg: Don't av_freep local variables

Message ID 20190522010441.44257-7-andreas.rheinhardt@gmail.com
State Rejected
Headers show

Commit Message

Andreas Rheinhardt May 22, 2019, 1:04 a.m. UTC 
There is no danger of leaving dangling pointers behind, as the lifespan
of local variables (including pointers passed (by value) as function
arguments) ends anyway as soon as we exit their scope.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/cbs.c       | 2 +-
 libavcodec/cbs_jpeg.c  | 8 ++++----
 libavcodec/cbs_mpeg2.c | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

Comments

Mark Thompson May 28, 2019, 11:09 p.m. UTC | #1 
On 22/05/2019 02:04, Andreas Rheinhardt wrote:
> There is no danger of leaving dangling pointers behind, as the lifespan
> of local variables (including pointers passed (by value) as function
> arguments) ends anyway as soon as we exit their scope.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/cbs.c       | 2 +-
>  libavcodec/cbs_jpeg.c  | 8 ++++----
>  libavcodec/cbs_mpeg2.c | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)

I don't agree with the premise of this patch.  I think it's sensible to use av_freep() everywhere, because the marginal cost over av_free() is ~zero and it makes some possible use-after-free errors easier to find.  (While all of these do appear just before the containing functions return, in general it's not useful to even need to think about that.)

- Mark
diff mbox

Patch

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index 0260ba6f67..fc2246970e 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -97,7 +97,7 @@  int ff_cbs_init(CodedBitstreamContext **ctx_ptr,
 
     ctx->priv_data = av_mallocz(ctx->codec->priv_data_size);
     if (!ctx->priv_data) {
-        av_freep(&ctx);
+        av_free(ctx);
         return AVERROR(ENOMEM);
     }
 
diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
index 5a72f0e2e7..b74747cbbf 100644
--- a/libavcodec/cbs_jpeg.c
+++ b/libavcodec/cbs_jpeg.c
@@ -86,21 +86,21 @@  static void cbs_jpeg_free_application_data(void *unit, uint8_t *content)
 {
     JPEGRawApplicationData *ad = (JPEGRawApplicationData*)content;
     av_buffer_unref(&ad->Ap_ref);
-    av_freep(&content);
+    av_free(content);
 }
 
 static void cbs_jpeg_free_comment(void *unit, uint8_t *content)
 {
     JPEGRawComment *comment = (JPEGRawComment*)content;
     av_buffer_unref(&comment->Cm_ref);
-    av_freep(&content);
+    av_free(content);
 }
 
 static void cbs_jpeg_free_scan(void *unit, uint8_t *content)
 {
     JPEGRawScan *scan = (JPEGRawScan*)content;
     av_buffer_unref(&scan->data_ref);
-    av_freep(&content);
+    av_free(content);
 }
 
 static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
@@ -227,7 +227,7 @@  static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
                                       data, data_size, data_ref);
         if (err < 0) {
             if (!data_ref)
-                av_freep(&data);
+                av_free(data);
             return err;
         }
 
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 4f89435c9d..df2e818f07 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -146,7 +146,7 @@  static void cbs_mpeg2_free_user_data(void *unit, uint8_t *content)
 {
     MPEG2RawUserData *user = (MPEG2RawUserData*)content;
     av_buffer_unref(&user->user_data_ref);
-    av_freep(&content);
+    av_free(content);
 }
 
 static void cbs_mpeg2_free_slice(void *unit, uint8_t *content)
@@ -154,7 +154,7 @@  static void cbs_mpeg2_free_slice(void *unit, uint8_t *content)
     MPEG2RawSlice *slice = (MPEG2RawSlice*)content;
     av_buffer_unref(&slice->header.extra_information_ref);
     av_buffer_unref(&slice->data_ref);
-    av_freep(&content);
+    av_free(content);
 }
 
 static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,