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

[Michael Niedermayer]

On Sun, Aug 02, 2020 at 08:40:27PM +0100, Jack Haughton wrote:
> Hello,
> 
> Apologies for the delay in replying. This patch is not to address a
> specific problem that currently exists, but rather to restore the
> robustness to invalid input that previously existed in this function. The
> motivation for the patch is that we would like to merge a500b975 into our
> local tree to gain support for gcc 10, but there are concerns about the
> removal of the null check. av_parse_video_rate() passes its argument
> directly to strcmp, which would cause undefined behaviour if the argument
> was NULL.

now it makes it even less sense to me.
av_parse_video_rate() doesnt support a NULL argument before or after your
patch, its undefined behaviour either way.
what you change is a private function using that public function.

Now if you changed the public function to do something specific with a NULL
argument then that would be more robust maybe but not when its done to a
single caller which should not pass NULL there

thx



> 
> Thanks,
> Jack
> 
> On Fri, Jul 31, 2020 at 8:31 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > Hi
> >
> > On Fri, Jul 31, 2020 at 03:53:56PM +0100, Jack Haughton wrote:
> > > Commit a500b975 removed NULL input handling from this function,
> > > moving the check higher up the call tree in one branch. However,
> > > there is another call to set_string_video_rate() which may pass
> > > NULL, and future users of the function may not be clear that
> > > a NULL check is required. This patch restores the NULL check to
> > > avoid these problems.
> >
> > Does this affect something else than the seting based on defaults ?
> > because the defaults should probably be valid values
> > or if you disagree, iam curious where the default would be intentionally
> > invalid
> >
> > thx
> >
> > [...]
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Breaking DRM is a little like attempting to break through a door even
> > though the window is wide open and the only thing in the house is a bunch
> > of things you dont want and which you would get tomorrow for free anyway
> >
> 
> 
> -- 
> 
> 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 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".

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.
-------------- 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/20200803/9667ba06/attachment.sig>