[FFmpeg-devel] [PATCH] avutil/opt: Restore NULL input handling to set_string_video_rate()

[Jack Haughton]

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.rheinhardt at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at 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