Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of James Almer > Sent: Friday, May 17, 2019 5:30 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx) > > On

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

May 16, 2019, 10:29 PM by jamr...@gmail.com: > On 5/16/2019 6:06 PM, Lynne wrote: > >> May 16, 2019, 8:43 PM by >> geo...@nsup.org >> : >> >>> Lynne (12019-05-16): >>> I'm not, I still want the 2 checks. >>> >>> Arguments please. As I explained, the first check

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

On 5/16/2019 6:06 PM, Lynne wrote: > May 16, 2019, 8:43 PM by geo...@nsup.org: > >> Lynne (12019-05-16): >> >>> I'm not, I still want the 2 checks. >>> >> >> Arguments please. As I explained, the first check is harmful to >> applications because it hides bug. >> > > Nevermind, its not really

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

May 16, 2019, 8:43 PM by geo...@nsup.org: > Lynne (12019-05-16): > >> I'm not, I still want the 2 checks. >> > > Arguments please. As I explained, the first check is harmful to > applications because it hides bug. > Nevermind, its not really possible to give av_tx_uninit() a NULL double pointer

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

May 16, 2019, 7:22 PM by jamr...@gmail.com: > On 5/16/2019 3:11 PM, Nicolas George wrote: > >> James Almer (12019-05-16): >> >>> There are two precedents in the codebase, one checking for both the >>> passed argument and then the struct pointer pointed by it (av_bsf_free >>> and av_buffer_unref

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

Lynne (12019-05-16): > I'm not, I still want the 2 checks. Arguments please. As I explained, the first check is harmful to applications because it hides bug. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

On 5/16/2019 3:11 PM, Nicolas George wrote: > James Almer (12019-05-16): >> There are two precedents in the codebase, one checking for both the >> passed argument and then the struct pointer pointed by it (av_bsf_free >> and av_buffer_unref as i mentioned above), and one checking only the >>

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

James Almer (12019-05-16): > There are two precedents in the codebase, one checking for both the > passed argument and then the struct pointer pointed by it (av_bsf_free > and av_buffer_unref as i mentioned above), and one checking only the > struct pointer (av_bsf_list_free as you mentioned in

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

On 5/16/2019 11:31 AM, Nicolas George wrote: > James Almer (12019-05-16): >> An assert is meant to detect developer errors, not user errors. Crashing >> the user's whole application because they misused the API is not really >> acceptable. >> >> I can't find examples of such functions using

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

James Almer (12019-05-16): > An assert is meant to detect developer errors, not user errors. Crashing > the user's whole application because they misused the API is not really > acceptable. > > I can't find examples of such functions using asserts this way, but > there are several

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

On 5/16/2019 5:03 AM, Nicolas George wrote: > Ruiling Song (12019-05-16): >> ctx is a pointer to pointer here. >> >> Signed-off-by: Ruiling Song >> --- >> libavutil/tx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavutil/tx.c b/libavutil/tx.c >> index

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

John Cox (12019-05-16): > >> -if (!ctx) > >> +if (!ctx || !(*ctx)) > >That would protect somebody stupid enough to call av_tx_uninit(NULL) > >instead of av_tx_uninit(). A hard crass is completely warranted in > >this case. An assert would be acceptable. > Actually that is what the original

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

May 16, 2019, 11:35 AM by geo...@nsup.org: > Lynne (12019-05-16): > >> LGTM >> > > As I said, not to me. Please take all comments into account. > Its fine by me. Adding an AVClass is unwarranted and absolutely no standalone API we've ever used like av_fft or av_has has a class.

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

>Ruiling Song (12019-05-16): >> ctx is a pointer to pointer here. >> >> Signed-off-by: Ruiling Song >> --- >> libavutil/tx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavutil/tx.c b/libavutil/tx.c >> index 934ef27c81..1690604040 100644 >> ---

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

Lynne (12019-05-16): > LGTM As I said, not to me. Please take all comments into account. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

May 16, 2019, 6:02 AM by ruiling.s...@intel.com: > ctx is a pointer to pointer here. > > Signed-off-by: Ruiling Song <> ruiling.s...@intel.com > > > > --- > libavutil/tx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavutil/tx.c

Re: [FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

Ruiling Song (12019-05-16): > ctx is a pointer to pointer here. > > Signed-off-by: Ruiling Song > --- > libavutil/tx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavutil/tx.c b/libavutil/tx.c > index 934ef27c81..1690604040 100644 > --- a/libavutil/tx.c > +++

[FFmpeg-devel] [PATCH V2] avutil/tx: add check against (*ctx)

ctx is a pointer to pointer here. Signed-off-by: Ruiling Song --- libavutil/tx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/tx.c b/libavutil/tx.c index 934ef27c81..1690604040 100644 --- a/libavutil/tx.c +++ b/libavutil/tx.c @@ -697,7 +697,7 @@ static int