Re: [FFmpeg-devel] [PATCH 1/5] lavu/common.h: Fix UB in av_clipl_int32_c() - Rémi Denis-Courmont

From: "Rémi Denis-Courmont" <remi@remlab.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/5] lavu/common.h: Fix UB in av_clipl_int32_c()
Date: Thu, 30 May 2024 17:28:55 +0300
Message-ID: <7744F895-46D2-471F-B9A6-821D507E6519@remlab.net> (raw)
In-Reply-To: <e1639aa077a55314654da05aba598b569fd84ba7.camel@haerdin.se>

Le 30 mai 2024 17:07:21 GMT+03:00, "Tomas Härdin" <git@haerdin.se> a écrit :
>> We should depend on punning as long as it conforms to the standard.
>
>My mistake, I forgot type punning is allowed in C. It's UB in C++
>
>> > The standard compliant way
>> > is to use memcpy()
>> 
>> That's way worse than union in terms of how proactively the compiler
>> needs to optimise, and both approaches are as confirming.
>
>A good compiler will do the same thing

True, and I don't care very much about memcpy vs union, as they both rely on matching representation. AFAIR, FFmpeg tends to use unions though.

>
>Maybe I can get the riscv version covered by Eva as well. That's beyond
>the scope of this patchset

IMHO, this specific patch (and the following one) are beating dead horses. Sure there may be theoretical UB in the current code, but if there is a *better* implementation, better switch to that than bike shedding the fix for the UB.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".