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 <ruiling.s...@intel.com> >> --- >> 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 gen_mdct_exptab(AVTXContext *s, int len4, >> double scale) >> >> av_cold void av_tx_uninit(AVTXContext **ctx) >> { > >> - if (!ctx) >> + if (!ctx || !(*ctx)) > > That would protect somebody stupid enough to call av_tx_uninit(NULL) > instead of av_tx_uninit(&var). A hard crass is completely warranted in > this case. An assert would be acceptable.
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 uninit/free/unref functions behave like the above patch. See av_buffer_unref(), av_packet_free(), av_bsf_free(). Other functions instead just don't even consider the passed argument could be NULL at all, like avcodec_free_context() and swr_free(), which while not 100% safe, is a pretty realistic expectation. I'd say either apply this patch as is, or apply the original one sent last night. In both cases it will be following an existing precedent in the codebase. > >> return; >> >> av_free((*ctx)->pfatab); > > Regards, > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".