diff mbox series

[FFmpeg-devel,v4,24/24] avutil/internal: remove FF_ALLOCx{_ARRAY}_OR_GOTO macros

Message ID 1591111618-15778-24-git-send-email-lance.lmwang@gmail.com
State Accepted
Commit 2658680df4fc606522e5f65899afb9a98b47d287
Headers show
Series [FFmpeg-devel,v4,01/24] avcodec/h264dec: cosmetics | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang June 2, 2020, 3:26 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Nicolas George comments for the macros:
These functions have a terrible design, let us fix them before extending
them.
First design mistake: no error code. A helper function for testing
memory allocation failure where AVERROR(ENOMEM) does not appear is
absurd.

Second design mistake: printing a message. Return the error code, let
the caller print the error message.

Third design mistake: hard-coded use of goto.

http://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262544.html

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavutil/internal.h | 36 ------------------------------------
 1 file changed, 36 deletions(-)

Comments

Lance Wang June 11, 2020, 10:51 a.m. UTC | #1 
On Tue, Jun 02, 2020 at 11:26:58PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Nicolas George comments for the macros:
> These functions have a terrible design, let us fix them before extending
> them.
> First design mistake: no error code. A helper function for testing
> memory allocation failure where AVERROR(ENOMEM) does not appear is
> absurd.
> 
> Second design mistake: printing a message. Return the error code, let
> the caller print the error message.
> 
> Third design mistake: hard-coded use of goto.
> 
> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262544.html
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavutil/internal.h | 36 ------------------------------------
>  1 file changed, 36 deletions(-)
> 
> diff --git a/libavutil/internal.h b/libavutil/internal.h
> index 00f1a57..b87cc6d 100644
> --- a/libavutil/internal.h
> +++ b/libavutil/internal.h
> @@ -140,42 +140,6 @@
>  #define FF_ALLOC_TYPED_ARRAY(p, nelem)  (p = av_malloc_array(nelem, sizeof(*p)))
>  #define FF_ALLOCZ_TYPED_ARRAY(p, nelem) (p = av_mallocz_array(nelem, sizeof(*p)))
>  
> -#define FF_ALLOC_OR_GOTO(ctx, p, size, label)\
> -{\
> -    p = av_malloc(size);\
> -    if (!(p) && (size) != 0) {\
> -        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> -        goto label;\
> -    }\
> -}
> -
> -#define FF_ALLOCZ_OR_GOTO(ctx, p, size, label)\
> -{\
> -    p = av_mallocz(size);\
> -    if (!(p) && (size) != 0) {\
> -        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> -        goto label;\
> -    }\
> -}
> -
> -#define FF_ALLOC_ARRAY_OR_GOTO(ctx, p, nelem, elsize, label)\
> -{\
> -    p = av_malloc_array(nelem, elsize);\
> -    if (!p) {\
> -        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> -        goto label;\
> -    }\
> -}
> -
> -#define FF_ALLOCZ_ARRAY_OR_GOTO(ctx, p, nelem, elsize, label)\
> -{\
> -    p = av_mallocz_array(nelem, elsize);\
> -    if (!p) {\
> -        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> -        goto label;\
> -    }\
> -}
> -
>  #include "libm.h"
>  
>  /**
> -- 
> 1.8.3.1
> 

I'll apply the patch set in next two days if no further comments still. Please help to
review if you think it's necessary.
Nicolas George June 11, 2020, 10:53 a.m. UTC | #2 
lance.lmwang@gmail.com (12020-06-11):
> I'll apply the patch set in next two days if no further comments still. Please help to
> review if you think it's necessary.

Please do not include my name in the commit message.

Regards,
Lance Wang June 11, 2020, 1:19 p.m. UTC | #3 
On Thu, Jun 11, 2020 at 12:53:01PM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12020-06-11):
> > I'll apply the patch set in next two days if no further comments still. Please help to
> > review if you think it's necessary.
> 
> Please do not include my name in the commit message.

Sure, I have removed below line from the commit message in local:
"Nicolas George comments for the macros:" 

> 
> Regards,
> 
> -- 
>   Nicolas George



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavutil/internal.h b/libavutil/internal.h
index 00f1a57..b87cc6d 100644
--- a/libavutil/internal.h
+++ b/libavutil/internal.h
@@ -140,42 +140,6 @@ 
 #define FF_ALLOC_TYPED_ARRAY(p, nelem)  (p = av_malloc_array(nelem, sizeof(*p)))
 #define FF_ALLOCZ_TYPED_ARRAY(p, nelem) (p = av_mallocz_array(nelem, sizeof(*p)))
 
-#define FF_ALLOC_OR_GOTO(ctx, p, size, label)\
-{\
-    p = av_malloc(size);\
-    if (!(p) && (size) != 0) {\
-        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
-        goto label;\
-    }\
-}
-
-#define FF_ALLOCZ_OR_GOTO(ctx, p, size, label)\
-{\
-    p = av_mallocz(size);\
-    if (!(p) && (size) != 0) {\
-        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
-        goto label;\
-    }\
-}
-
-#define FF_ALLOC_ARRAY_OR_GOTO(ctx, p, nelem, elsize, label)\
-{\
-    p = av_malloc_array(nelem, elsize);\
-    if (!p) {\
-        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
-        goto label;\
-    }\
-}
-
-#define FF_ALLOCZ_ARRAY_OR_GOTO(ctx, p, nelem, elsize, label)\
-{\
-    p = av_mallocz_array(nelem, elsize);\
-    if (!p) {\
-        av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
-        goto label;\
-    }\
-}
-
 #include "libm.h"
 
 /**