Re: [FFmpeg-devel] [PATCH 00/22] Deprecate av_uninit

[Zhao Zhili <quinkblack-at-foxmail.com@ffmpeg.org>]

> On Apr 11, 2025, at 17:32, Nicolas George <george@nsup.org> wrote:
> 
> Zhao Zhili (HE12025-04-11):
>> With UB, the compiler can remove branch check and assign some random
>> value to it, which cannot be detected by valgrind.
>> 
>> For ab792634197e, the UB is there for decades and never detected by
>> valgrind, and the warning is silenced by av_uninit.
> 
> You make a valid point, but my own point still stands: your change make
> it harder to find bugs by removing the difference between “I put this
> initialization there because it is the right value” and “I put this
> there because the compiler is too stupid to figure it on its own”.
> 
> I have to ideas to accommodate both issues:
> 
> - Have a FATE instance with valgrind and optimizations disabled, so that
>  the compiler does not optimize the code away because of the UB and
>  valgrind catches the bug.
> 
> - Initialize to 0xDEADBEEF and add av_assert2(val != 0xDEADBEEF) at the
>  place where it will be used.

This is only check on particular compiler implementation. It works until someone
use a different compiler and UB kicks in.

> 
> The second one might be fragile, but less than an UB.
> 
>> By the way, logic bug isn’t equal to UB, so I’m not hiding UB.
> 
> Yes you are.
> 
>> Who put av_uninit in the code means there is no logic bug. If there is,
>> the patchset fixed UB or replaced UB by deterministic logic error, which
>> can’t be worse.
> 
> The deterministic logic error is not worse, but it is not better either,
> and it is harder to detect.
> 
>> If there is, the patchset fixed or makes the issue deterministic.
> 
> This patches fixes NOTHING, I hope we agree on it. What you did is
> basically equivalent to removing an assert that fails and hoping the
> code that comes later will be fine.
> 
> Making the bug deterministic is not better if it makes it harder to
> detect.
> 
>> We don’t initialized all variables when declaration. But if there is a
>> sometimes-uninitialized warning, there is some reason for compiler.
>> Uninitialized warning isn’t the same as deprecated or unused, it should
>> never be ignored in my opinion.
> 
> You are wrong on this, the compiler is often unable to figure out that
> the code always sets the variable before reading it when the developer
> can prove it. Compilers are getting better, but they are still far from
> perfect, and av_unused is precisely there for that lack of perfection,
> and needs to stay there.
> 
> Regards,
> 
> -- 
>  Nicolas George
> _______________________________________________
> 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".

_______________________________________________
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".