On Fri, Aug 14, 2015 at 5:14 AM, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Thu, Aug 13, 2015 at 08:57:12PM -0400, Ganesh Ajjanagadde wrote: >> On Thu, Jul 30, 2015 at 4:06 PM, Ganesh Ajjanagadde >> <gajjanaga...@gmail.com> wrote: >> > Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com> >> > --- >> > libavcodec/aacdec_template.c | 5 ----- >> > 1 file changed, 5 deletions(-) >> > >> > diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c >> > index 2f270bc..d7849da 100644 >> > --- a/libavcodec/aacdec_template.c >> > +++ b/libavcodec/aacdec_template.c >> > @@ -1907,11 +1907,6 @@ static int decode_ics(AACContext *ac, >> > SingleChannelElement *sce, >> > ac->oc[1].m4ac.object_type == AOT_ER_AAC_LD || >> > ac->oc[1].m4ac.object_type == AOT_ER_AAC_ELD; >> > >> > - /* This assignment is to silence a GCC warning about the variable >> > being used >> > - * uninitialized when in fact it always is. >> > - */ >> > - pulse.num_pulse = 0; >> > - >> > global_gain = get_bits(gb, 8); >> > >> > if (!common_window && !scale_flag) { >> > -- >> > 2.5.0 >> > >> >> I guess I should have clarified the intent of this patch: >> This particular code only existed (as shown in the comment) to work >> around a compiler bug regarding diagnostics. >> Said compiler bug does not affect GCC of any recent vintage (I run the >> latest 5.2, but tested 4.8.1), > > many people and also developers still use older compilers > false positive warnings distract from real issues
There is a balance that we strike here: some devs use new, shiny compilers that may have bugs fixed (but may introduce new bugs of their own, see e.g the -Warray-bounds stuff), some others may use old versions requiring the workaround. It is a matter of subjective interpretatition as to what workarounds are ugly and which are not. I consider this one not that bad, so without information on when it was triggered/fixed, I agree with you. Nevertheless, see the analysis below. > > do you know in which version of gcc this was fixed? I can only speculate here. The trouble is -Wuninitialized has too many instances on the bug tracker, and I can't pinpoint it. So I tested GCC down to 4.7 (not a complete search, but tested all feature releases 4.x.0). The warning is not triggered. Keep in mind that this hack was added a long time back in commit 9cc04edff9 dating to Aug 2008. This is roughly the GCC 4.2-4.3 era. Note also the release notes for GCC 4.4.x tell us that heavy work was done on -Wuninitialized, and in GCC 4.5.2 regressions from 4.4 were cleaned up: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48028. In summary, I would estimate somewhere between 4.3 and 4.5. I can't do further work, since those versions of GCC don't even build with their latest compiler unless I string together a bunch of hacks/patches. I think in future such hacks should have the version number of the tool in the comment, it is trivial work to report version number (e.g gcc -v), helps a lot with removing ugly hacks at a later date, and can be easily done while reviewing patches. The current comment only informs us of a GCC problem, but not when. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > it is not once nor twice but times without number that the same ideas make > their appearance in the world. -- Aristotle > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel