I built McIDAS using GCC and sorted the resulting error messages. Below are descriptions of the most frequent warning messages which turned up.
char *
pointer. Compilers may store constant strings
in read-only, to guarantee that they aren't changed. If
you assign a constant string to a char *
you're risking
that somebody will pass that string to a routine which will
attempt to alter it and cause a core-dump.
The best fix is to make sure the pointer is a const char *
(which may turn up one or more functions which should take a
`const' pointer but don't. These should also be fixed, which
might turn up more functions which need to be `const'ified,
etc.
Even though this is a lot of work, it's worth the effort since many compilers will be able to do better optimization with variables whose values are guaranteed not to change.
If possible, change the definition of the called function so that it properly handles a `const' pointer. This can
If you can't change the function, another approach is to make a copy of the value and pass that to the (potentially dangerous) function.
If a function is used in more than one file, there should be a prototype for it in a header file somewhere. This keeps functions and their uses from getting out of sync
If the function is only used in this file, make it static
to
If they're used in #ifdef
'd code, you can either #ifdef
the
variable declaration as well, or look at the code and see
if both the variable declaration and the #ifdef
'd code
can be deleted.
const char *
to a char *
so they don't have to deal with it.
func(unsigned a)
with an int
variable)
don't blindly add casts (like func((unsigned )longVar)
),
to silence the warning, because you may just be hiding a
bug.
If, for instance, func()
expects a string length and
someone wrote:
int len = read(fd, thisBlock, sizeof(thisBlock)); if (strncmp(target, thisBlock, len) == 0) { printf("Found it\n"); }GCC would warn that it was passing the
len
argument to
strncmp()
as an unsigned variable due to its prototype.
This would work until read()
hit the end of the file,
when it would return a -1
, which would be passed as
(unsigned )-1
, which translates to some very large number
which is certainly far past the end of either chunk of data.
The odds are fairly good that strncmp()
will find a difference
before it walks off the end of a block into coredump territory,
but it is possible.
Instead of just casting len
to unsigned
, the proper fix
would be to check for -1
and deal with the error.
unsigned
if you can GUARANTEE that the signed value will
never be negative, `signed' if you can GUARANTEE that the
unsigned value won't be larger than CHAR_MAX
/SHRT_MAX
/INT_MAX
/etc.
It is possible, though, that there are multiple instances of a prototype for the same function, in which case you should probably get rid of all but the first prototype instance.
foo
is an external function and you
need to include the proper header file in order to bring in
the appropriate prototype, or that foo
is an internal function
but you're using it before it's defined, in which case you can
either move the definition to someplace before the first use
or you can add a local prototype at the top of the file.
Ignore this if it's in multiple system header files.
const void *
arguments ... if, for some wierd reason, a
comparison function never needs to look at the second pointer,
it still needs to include it or things might possibly break.
Usually, you just need to change the local variable name slightly to make this go away.
Sometimes, however, this points out a bigger problem. I've seen programs like this:
int func(struct item *items, size_t numItems) { int i; for (i = 0; i < numItems; i++) { ...a bunch of code... if (x > 0) { int i; for (i = 0; i < x; i++) { ...a bunch more code... items[i].thing = x; /* OOPS */ ...Here, someone added an inner block which uses a locally-declared instance of
i
to loop through some values, and someone later
added the "OOPS" line, expecting that they were using the outer
instance of i
rather than the inner instance.
Fix if possible, otherwise it's probably OK to cast these away.
Mcprintf()
, Mcsprintf()
, and Mceprintf()
).
This warns about code where the format doesn't match the
appropriate argument's datatype.
There's not much you can do about this, and it's probably not worth your time to try to avoid these warnings anyway.
If I can figure out how to deactivate these, I will.
struct
type like
struct foo var = { 0, 17, 12 };
but they've either missed one or more components or one or more
components have been added.
The compiler will initialize the extra component(s) to 0 (or the equivalent), but it's best to be explicit, so the problem will be immediately apparent if an extra component is inserted into the middle of the structure.
printf()
doesn't deal well with embedded NUL characters.
As you might guess, the fix is just to take out the \0