Hi,
I split this out in individual patches. Some are clearly good to get
cleaned up and get in immediately. But some might need some discussion
first. The four I think are fine and would like to commit are attached.
On Fri, 2015-09-04 at 12:04 -0700, Chih-Hung Hsieh wrote:
> * Replace K&R function definition with prototypes to match their
> declarations.
> Clang gives errors of: promoted type 'int' of K&R function parameter
> is not compatible with the parameter type
This is OK. I would like to get rid of the K&R function definitions in
general. They can hide some issues (see also below). We should probably
use gcc -Wold-style-definition to find them all.
I am not really sure why just these few get flagged. Most cases seem to
be flagged because there are no explicit prototypes. In the backends
case the _init functions are properly called through the ebl_bhinit_t in
openbackend. There are no explicit prototypes for these functions
though.
In the libasm FCT and UFCT case the various addintXX.c files include the
asm_addint8.c file to generate the various variants, the files don't
include any definitions.
In the case of asm_begin, dwarf_next_cfi, __libdw_intern_next_unit,
__libdw_findcu I that the warning is because they all have a 'bool'
argument that might be promoted differently in pre-ansi code?
I don't understand why there is a complaint about
ebl_openbackend_machine () and ebl_check_st_other_bits (). Those do look
fine to me with a declaration from libebl.h which is included. The issue
might again be that the last arguments might be promoted differently.
If we want to get rid of the K&R function definitions then lets start
with these. I reformatted them a little so they confirm with the GNU
coding standards we use.
> * Add const declaration to locs, which was passed a const.
> Clang gives errors of: passing 'const Elf_Data *' to parameter of
> type 'Elf_Data *' discards qualifiers
Good. This was also caused by a K&R function definition. I changed it
also to a new style definition and gcc warns about this too.
> * Avoid clang errors of: comparison of nonnull parameter ... equal to a null pointer is false
> on first encounter [-Werror,-Wtautological-pointer-compare]
> The parameter was declared as non-null.
If they are marked as nonnull arguments then I think we should just
remove the NULL checks. Casting away to get rid of the warning seems the
wrong approach.
> * Replace abs with labs for int64 values.
Nice catch. But labs is for longs, which might on some arches be only 32
bits. Should we be using llabs?
> * Remove unused static variables.
This is fine. They are clearly unused.
BTW this is gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=28901
> * Init local variable before use, where static analysis failed.
OK, because it is just test code. But in general it seems bad to
unnecessary initialize variables.
Thanks,
Mark