[FFmpeg-devel] How to handle compiler warnings

[Michael Niedermayer]

On Mon, Feb 04, 2008 at 12:50:31AM +0100, Diego Biurrun wrote:
> On Mon, Feb 04, 2008 at 12:18:27AM +0100, Michael Niedermayer wrote:
> > On Sun, Feb 03, 2008 at 10:58:04PM +0100, Diego Biurrun wrote:
> > > On Thu, Jan 31, 2008 at 03:28:11PM +0100, Michael Niedermayer wrote:
> > > > On Thu, Jan 31, 2008 at 10:19:56AM +0100, Diego Biurrun wrote:
> > > > > On Tue, Jan 29, 2008 at 11:45:15AM +0100, Diego Biurrun wrote:
> > > > > > The topic has come up again, it's time to discuss the subject.  I
> > > > > > propose to try to avoid compiler warnings as much as possible in order
> > > > > > to
> > > > > > 
> > > > > > - have cleaner code,
> > > > > > - have important warnings not be drowned out,
> > > > > > - make FFmpeg a programming textbook.
> > > > > > 
> > > > > > This does not include warning fixes that slow things down or obfuscate
> > > > > > the code, but if in doubt I personally would err on the side of fixing
> > > > > > the warning.
> > > > > 
> > > > > OK, we pretty much seem to have consensus about this.  Should we add a
> > > > > paragraph about warnings to the policy?
> > > > 
> > > > yes
> > > 
> > > Like this?
> > 
> > [...]
> > >  @item
> > > -    Do not change code to hide warnings without ensuring that the underlying
> > > -    logic is correct and thus the warning was inappropriate.
> > > +    Compiler warnings should be avoided unless the warning fix causes a
> > > +    slowdown or obfuscates the code.
> > >  @item
> > 
> > The sense behind warnings is to point to potential bugs or code with bad
> > style. If a type of warning would always point to correct and clean code, that
> > warning should be disabled not the code changed.
> > Thus the remaining warnings could point to bugs or correct code. First one has
> > to find out which of the 2 it is. If it is a bug, the bug should be fixed. If
> > it is correct code, it should be changed so it does not generate a warning
> > unless that causes a slowdown or obfuscates the code.
> 
> Is this better?

[...]
>  @item
> -    Do not change code to hide warnings without ensuring that the underlying
> -    logic is correct and thus the warning was inappropriate.
> +    Compiler warnings should be fixed if they point to real bugs. If not
> +    the code should be changed so it does not generate warnings unless
> +    this causes a slowdown or obfuscates the code.
>  @item

IMHO not at all
I dont understand why you remove that point from the policy.

We voted about adding a point which says remove warnings where it doesnt cause
a slowdown or obfuscates the code. That is it would be a reason to reject a
patch if it generated new easily avoidable warnings.

The problem i have with your formulation is that
"Compiler warnings should be fixed if they point to real bugs"
You here concentrate on the warning but thats totally wrong, if theres a
bug, the bug should be fixed, the warning is not relevant. If after fixing
the bug there still is a warning left, that is a seperate issue to deal with.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080204/d1dc50d7/attachment.pgp>