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

[James Almer]

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 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.
> 
> Re-read the code more carefully, and look what the existing predecent
> does. You made the same mistake as me.
> 
> Regards,

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 another email).

The current code is wrong as already established. This patch applying
(!ctx || !*ctx), and the one sent last night applying (!*ctx) are both
correct, each of them implementing one of the two aforementioned
precedents. The former just takes extra precautions against user error.