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 another email).
There are a few other precedent. I think the majority dereference the pointer and only accept indirect NULL. This is not surprising: freeing something unconditionally is very useful when uniniting after a failure case or different branches of programming. On the other hand, freeing nothing is not a useful pattern. > 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 Ok, I had missed there was another patch. > precedents. The former just takes extra precautions against user error. Not user: application programmer. This is a very important difference: users are supposed clueless, application programmers are supposed competent, even when they are not. That kind of mistake in using the API is probably a simple but grave mistake in the code, like writing av_foo_free(foo) instead of av_foo_free(&foo) (and ignoring the resulting warning). If ignored, it will lead to invalid frees or memory leaks in the application. This is not a service for the programmer: better have the program crash hard (either with an assert or a NULL deref) immediately, so they can fix it. Regards, -- Nicolas George
signature.asc
Description: PGP signature
_______________________________________________ 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".
