Jack Haughton (12020-08-04): > Absolutely, he should fix his code. But let's say some code gets by code > review that has a corner case whereby NULL can be passed. Isn't it better > for that condition to be handled cleanly (as it was before a500b975) rather > than causing undefined behaviour? Then the error will be reported to the > user with a clear error message and can be diagnosed and fixed quickly. > Currently, what happens in this case will be implementation-dependent, > making it much more difficult to diagnose.
It depends on what kind of undefined behavior we are talking about. Here, it is about dereferencing NULL, which always result in a segmentation fault, and if we really want to make sure, we can force it with an assert. What you are advocating is a typical example of what is called "defensive programming". When there is some invariant that the code is supposed to enforce (here: a pointer that should not be NULL), defensive programming involves making provisions so that the programs continues even if that invariant is broken. The thing is: defensive programming is almost always wrong. If an invariant is broken, that means there is a bug. We do not know which bug, but there is a bug. And since we do not know what it is, we have to assume the worst is possible: exploitable security issue, silent data corruption. And even if the worst does not happen, defensive programming makes debugging harder: it hides the bugs, let the program seems to work, or it delays the manifestation of the bug, and therefore requires more work to track it. Also note that people will (irrationally) be in more hurry to fix a crash than to fix an error message that looks clean. In this kind of cases, we have to ask ourselves a series of questions: 1. Is it convenient to let the caller pass NULL and have a sane result? → For free(), yes. → For av_parse_video_rate(), no, so we go to the next question. 2. Is it easy for the caller to check the validity of the parameter? → For == NULL, yes. Then the correct way of dealing with it is (1) document "the parameter must not be NULL", (2) make sure it crashes early if the parameter is NULL. 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".
