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

[Michael Niedermayer]

On Mon, Aug 03, 2020 at 02:07:36PM +0100, Jack Haughton wrote:
> 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

who is passing NULL in which way ?
this is not a public function

You can achieve such a NULL input by seting up an invalid AVOption
but you dont seem to speak about that. 

please explain what sort of robustness you want to achieve here
Is this about something iam missing or a lib user passing invalidly
setup AVOptions ? (he should fix his code) or some FFmpeg internal
robustness or what else ?


> undefined behaviour, where previously it would have been cleanly handled
> with an explicit error code. I'd contend that this is a bad response.


thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200804/509ebb76/attachment.sig>