> On Apr 11, 2025, at 17:32, Nicolas George <geo...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
