Hi Andreas, You're right - apologies. So in fact there was an exchange of one kind of undefined behaviour for another.
Thanks Jack On Thu, Aug 6, 2020 at 1:52 PM Andreas Rheinhardt < andreas.rheinha...@gmail.com> wrote: > 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". -- Argon Design Ltd., Registration No. 06977690, a Broadcom Inc. company Registered in England with registered office at St John's Innovation Centre, Cowley Road, Cambridge, CB4 0WS _______________________________________________ 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".
