On Wed, Jul 03, 2019 at 09:41:41AM +0200, Reimar Döffinger wrote: > > > On 03.07.2019, at 08:29, Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > On Tue, Jul 02, 2019 at 08:42:43PM -0300, James Almer wrote: > >> On 7/2/2019 5:56 PM, Michael Niedermayer wrote: > >>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > >>> --- > >>> doc/APIchanges | 3 +++ > >>> libavutil/mem.h | 13 +++++++++++++ > >>> libavutil/version.h | 2 +- > >>> 3 files changed, 17 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/doc/APIchanges b/doc/APIchanges > >>> index b5fadc2a48..65b8ed74ad 100644 > >>> --- a/doc/APIchanges > >>> +++ b/doc/APIchanges > >>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >>> > >>> API changes, most recent first: > >>> > >>> +2019-07-XX - XXXXXXXXXX - lavu 56.31.100 - mem.h > >>> + Add av_memcpy() > >>> + > >>> 2019-06-21 - XXXXXXXXXX - lavu 56.30.100 - frame.h > >>> Add FF_DECODE_ERROR_DECODE_SLICES > >>> > >>> diff --git a/libavutil/mem.h b/libavutil/mem.h > >>> index 5fb1a02dd9..a35230360d 100644 > >>> --- a/libavutil/mem.h > >>> +++ b/libavutil/mem.h > >>> @@ -506,6 +506,19 @@ void *av_memdup(const void *p, size_t size); > >>> */ > >>> void av_memcpy_backptr(uint8_t *dst, int back, int cnt); > >>> > >>> +/** > >>> + * memcpy() implementation without a NULL pointer special case > >>> + * > >>> + * @param dst Destination buffer > >>> + * @param src Source buffer > >>> + * @param cnt Number of bytes to copy; must be >= 0 > >>> + */ > >>> +static inline void av_memcpy(void *dst, const void *src, size_t cnt) > >> > >> How many cases are there in the codebase where cnt can be 0, and dst or > >> src NULL, without it having been checked before calling memcpy? And from > >> those, which would not be from situations where the code should have > >> instead aborted and returned ENOMEM, or EINVAL if either of them are > >> function arguments? > > > > There are around 2500 occurances of memcpy in the codebase > > To awnser your question it would be needed to review all of them and in many > > cases their surrounding code. > > So that is unlikely to be awnsered by anyone accuratly > > > > Also iam not sure i understand why you ask or why this would matter > > the suggested function allows to simplify cases where the NULL can > > occur, not where it cannot or should not. That is this is intended for > > the cases where we already have or are adding explicit checks to > > avoid the NULL case. > > > > i could rename this to av_memcpy_nullsafe which would make it clearer but > > also its more to write and read > > I admit I thought that a worthwhile idea originally, > but I have to think back to a long time ago that every function added to our > "API" has a cost of people having to know about it and how to use it. > And if it's currently only 2 places that would benefit I think James is right > to ask if it makes sense. > Of course another question might be if it might make sense to replace all > memcpy uses with it. > I mean, isn't it naturally expected behaviour that the pointers would be > ignored if the copy amount is 0? There might be a lot of code assuming that > we do not know about...
in addition to the 2 there are these related commits found by very dumb git log greps In further addition there would be cases that deal with src == dst, something we could add a check for in av_memcpy() too commit c6aaf0840cf9b2b8cb139ed7110d3d47c2bf3d12 Author: Carl Eugen Hoyos <ceho...@ag.or.at> Date: Tue Apr 18 10:56:31 2017 +0200 lavf/mov: Only copy extradata if it exists. Avoids undefined call of memcpy(ptr, NULL, 0); commit fde9013ab42411ee2015811c28e8921828a81702 Author: Derek Buitenhuis <derek.buitenh...@gmail.com> Date: Thu Jul 6 13:23:06 2017 -0400 mpegtsenc: Don't pass NULL to memcpy Signed-off-by: Derek Buitenhuis <derek.buitenh...@gmail.com> commit 7bab631f7df55b361368296f125b95a1814bc18c Author: Michael Niedermayer <michae...@gmx.at> Date: Wed Mar 6 01:37:49 2013 +0100 mss2dsp/upsample_plane: fix 0x0 handling Fixes invalid memcpy and out of array accesses Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind Signed-off-by: Michael Niedermayer <michae...@gmx.at> commit c54eef46f990722ed65fd1ad1da3d0fc50806eb5 Author: Carl Eugen Hoyos <ceho...@ag.or.at> Date: Thu Sep 22 01:03:55 2016 +0200 lavc/avpacket: Fix undefined behaviour, do not pass a null pointer to memcpy(). Fixes ticket #5857. commit f077ad69c682c13ab75a72aec11a61cac53f0c91 Author: Carl Eugen Hoyos <ceho...@ag.or.at> Date: Sun Sep 4 21:11:02 2016 +0200 lavc/avpacket: Fix undefined behaviour, do not pass a null pointer to memcpy(). Fixes ticket #5128. -- 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.
signature.asc
Description: PGP signature
_______________________________________________ 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".