Jack Haughton: > A NULL check in av_parse_video_rate() would certainly not be a bad idea. It > wouldn't (on its own) restore the NULL input robustness to > set_string_video_rate() though, as any error value returned from > av_parse_video_rate() would result in val being logged using %s, which is > the whole problem that a500b975 was aiming to solve. > > av_parse_video_rate() is also called from a lot more places (most of which > already have an explicit NULL check immediately before the call), so > changing the behaviour of that function by adding a NULL check potentially > has a lot more knock-on effects than simply restoring the previously > existing robustness to set_string_video_rate(), which is also in keeping > with the other av_parse_video_rate() callsites. > > You say that the caller 'should not' pass NULL to set_string_video_rate(). > Absolutely, agreed, but the point of checks is to handle unexpected > situations. If someone does pass NULL, as of a500b975 the response will be > undefined behaviour, where previously it would have been cleanly handled > with an explicit error code. I'd contend that this is a bad response. > You are contradicting yourself here: As the commit message of a500b975 makes clear and as you explain in the first paragraph, a NULL value would not have been handled cleanly before a500b975: Using NULL for %s is undefined behaviour, too. Just wanted to mention it because you repeat this point later in another mail:
Jack Haughton: > Isn't it better for that condition to be handled cleanly (as it was > before a500b975) rather than causing undefined behaviour? - Andreas _______________________________________________ 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".
