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. _______________________________________________ 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".
