[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: Compile warning using additonal CFLAGS '-Wshadow -Wpointer-arith -Wcast-qual -W'

John Smith writes:
> Ive just downloaded and compiled openldap-2.4.10, but when I add the
> CFLAGS '-Wshadow -Wpointer-arith -Wcast-qual -W' I get the following
> warnings that may require further investigation of an developer :

We should get around to clean away more warnings, but most are OK.

> - cast discards qualifiers from pointer target type

Usually that's what the cast is for.

> - missing initializer

Typically null-filled structs.  E.g. initial value of a fake Operation
struct, or the final element of a "null struct"-terminated array.

Still, we can #define null initializers for them.  A bit messy with
structs that have #ifdeffed members, but the #ifdef can #define the
part of the initializer for that part of the struct.

> - signed and unsigned type in conditional expression
> - comparison between signed and unsigned
> - comparison of unsigned expression >= 0 is always true
> - comparison of unsigned expression < 0 is always false

Yes, we should probably have a look at these.
... I don't like the WHATSLEFT compares in limits.c.

Some are fine, for datatypes which can or could be signed on some hosts
but are unsigned on others.  Or types which once were signed on some
hosts - I think size_t was signed somewhere (Sun?) when ANSI C was new.
Maybe it's about time to trust size_t to be unsigned...

It's very easy to make mistakes with signedness though, so any changes
have to be done carefully.

> - declaration of 'foo' shadows a previous local

Should clean up, but it's a typical "argh, I don't want this job" issue,
at least for big functions.

After all, the point of a warning is to catch potential errors, so I
think the right way to silence those is to strongly suspect there _is_
an error and try to find it.  Which means studying each function
carefully, and most likely not find anything.  Otherwise renaming one of
the variables will just make any errors harder to find and understand.

> - `foo' is deprecated; use `foobar' or `foo_r' instead

Yes, sys_nerr/sys_errlist.  The strerror code needs to be cleaned up.
We'll get around to it - someday:-(