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