[FFmpeg-devel] [PATCH] aacdec_template: remove obsolete warning suppression

[Ganesh Ajjanagadde]

On Fri, Aug 14, 2015 at 5:14 AM, Michael Niedermayer
<michael at 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
>> <gajjanagadde at gmail.com> wrote:
>> > Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at 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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>