[Flac-dev] FLAC_index

Erik de Castro Lopo erikd-flac at mega-nerd.com
Mon Sep 3 19:55:47 PDT 2007


Chris Arena wrote:

> Christian Weisgerber <naddy at mips.inka.de> wrote:
> 
> > I guess the use of unsigned index variables was an accident.  Changing
> > them back to int makes FLAC pass the "thorough" test suite on amd64.
> 
> I was wondering why there isn't a FLAC_index (FLAC_index16 or FLAC_index32)
> type that would prevent someone from just "winging" it with a bad choice for
> an index to an array?

Quite honestly, I don't think having an index type is going to do anything
helpful here. Firstly, there will be times when signed indexes are needed:

    for (i = -5 ; i <= 5 ; i++) ....

and other times when unsigned indexes are needed. Furthermore, just because
you have a FLAC_idex type doesn't prevent the C compiler from automatically 
coercing a FLAC_index type to and from signed or unsigned int.

The *only* thing that will prevent these kinds of problems is enabling
as many GCC warning flags as possible, having a strict policy of always
fixing compiler warnings and using -Werror (which turns warnings into
error) during development.

For instance, for the problem of conversions/compares that mismatch
types or specifiers -Wconversion and -Wsign-compare.

For my own stuff, I consider -Wall -Wextra an absolute minimum. I 
usually add -Werror while I'm actively working on it. 

I once tried enabling warnings on the FLAC code and found a relatively
large number of warnings. I tried to fix some of them, but many were
really difficult, like casts from "const char *" to "char *" because
of API decisions and pointer aliasing warnings. I think the later 
(pointer aliasing) may actually cause the compiler to generate slower 
code because it has to avoid certain optimisations when aliasing is 
detected.

If Josh was interested in a concerted effort to increase the number of
warnings turned on by default and fix all warnings found, I would be
happy to voulnteer to help.

Cheers,
Erik
-- 
-----------------------------------------------------------------
Erik de Castro Lopo
-----------------------------------------------------------------
"[T]hose who study jihad will understand why Islam wants to conquer the
whole world. All the countries conquered by Islam or to be conquered in
the future will be marked for everlasting salvation. For they shall live
under [God's law]."
--Ayatollah Ruhollah Khomeini: Islam Is Not a Religion of Pacifists (1942)


More information about the Flac-dev mailing list