[FFmpeg-devel] [PATCH 1/5] lavu/common.h: Fix UB in av_clipl_int32_c()

[James Almer]

On 5/30/2024 12:32 PM, Tomas Härdin wrote:
> tor 2024-05-30 klockan 17:28 +0300 skrev Rémi Denis-Courmont:
>>
>>
>> Le 30 mai 2024 17:07:21 GMT+03:00, "Tomas Härdin" <git at 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.
> 
> Are you saying that UB is acceptable? You know the compiler is free to
> assume signed arithmetic doesn't overflow, right? If so then what other
> UB might we accept?

He did not say that... He said we should switch to a better 
implementation rather than trying to fix the existing potentially buggy one.