James Almer: > On 2/14/2021 6:28 PM, Andreas Rheinhardt wrote: >> This is undefined behaviour in C, so use data = len ? data + len : data >> instead if data += len. GCC optimizes the branch away in this case; >> Clang unfortunately doesn't. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> >> --- >> Checking for len != 0 instead of > 0 allows the compiler to optimize the >> branch away; maybe future versions of Clang (I used 11) will do so, too. >> >> libavformat/utils.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/utils.c b/libavformat/utils.c >> index 3e955b85bc..cea6d4ca92 100644 >> --- a/libavformat/utils.c >> +++ b/libavformat/utils.c >> @@ -1426,7 +1426,7 @@ static int parse_packet(AVFormatContext *s, >> AVPacket *pkt, >> pkt->pts = pkt->dts = AV_NOPTS_VALUE; >> pkt->pos = -1; >> /* increment read pointer */ >> - data += len; >> + data = len ? data + len : data; > > Isn't adding any offset to a NULL pointer UB? That was afaik the entire > point behind the change in c40d36076a. > Yes, it is. And if it happened here, it would indicate a real bug (i.e. a real logic error) that would need to be fixed; but as long as it doesn't happen, we should apply cheap solutions that the compiler can optimize away (you probably notice that I regard NULL + 0 as not a real bug, but rather as an annoyance of the C standard that disallows this (even the sanitizer authors seem to agree with this point (that's why there is an error message specifically for NULL + 0); it is allowed in C++).
- 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".
