[vorbis-dev] Re: [vorbis] Re: Different average bitrates on different machines

[Segher Boessenkool]

[moved to vorbis-dev at xipf from vorbis at xiph]

Cameron Simpson wrote:
> 
> On Mon, Dec 10, 2001 at 03:30:24AM -0500, Monty <xiphmont at xiph.org> wrote:
> | > BTW, has anyone done a scan of the source with gcc 3.x with the
> | > -Wsequence-point warning enabled? There's been a fair bit of discussion
> | > recently about dodgy code doing things like:
> | >     a[i]=f(i++)
> | > which gives undefined results. That really could screw up encoding algorithms.
> | That's not 'dodgy', it's an outright error.  No C programmer should
> | ever even think of the above code being acceptable.
> 
> Naturally not, but sometimes they creep in. There was a thread in the
> Linux-Kernel list about such code just recently. Since gcc-3.x has a
> sequence point checker for this kind of issue, I'll make a pass over
> the code with it on just on principle sometime this week (since I have
> to rebuild oggenc et al to look for a bug anyway).

I normally compile with

-Wall -W -pedantic -std=c9x -Wtraditional -Wpointer-arith -Wconversion
-Wsign-compare

and boy, libvorbis looks really unclean, then.

-pedantic -std=c9x  makes everything not strictly ANSI C99 a warning
(we're supposed to be C99).

-Wall  catches common errors.

-W  catches even more errors (it sometimes bitches about perfectly valid
comstructs, but small code rearrangements will normally fix those
warnings.

-Wtraditional  warns about stuff that's not the same in ANSI C as it is
in K&R C;  this helps portability, but it complains about float function
parameters (these are promoted to double in K&R C).

-Wpointer-arith  should _always_ be enabled, and be an error.  Better yet,
it should hunt down the programmer who wrote that code and do bad things to
him.  No further comments.

-Wconversion  is really important.  It warns about dangerous type conversions
(dangerous in the sense that the compiler might not be doing what the programmer
thinks the compiler whould do).  This catches oodles of signed <-> unsigned
mismatches, for example.

Especially this last warning option generates a lot of warnings on the
Vorbis code.
Fixing this is not completely trivial, though.  Quite a number of
objects which
really should have unsigned type, have signed type in libvorbis, because negative
values are used for signalling special conditions.  I think this is a
bad thing.

Having objects of "natural" unsigned type (sizes, array offsets, most
loop counters,
are a few examples) actually _be_ unsigned type, enables the compiler to
do a much
better job of optimizing.  And it catches actual bugs, and it catches a
lot of
wacky things which ain't actual bugs, but only not because the actual
range of the
possible values is smaller than the type.

Hey, I'm not complaining;  but maybe someone might want to clean up a
little? :-)

Cheers,

Segher

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to 'vorbis-dev-request at xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is needed.
Unsubscribe messages sent to the list will be ignored/filtered.