Menu
▾
▴
Re: a bunch of warnings
|
From: Daniel J S. <dan...@ie...> - 2012-08-28 06:52:50
|
On 08/27/2012 11:21 PM, Ethan Merritt wrote:
> On Monday, 27 August 2012, Mojca Miklavec wrote:
>> Hello,
>>
>> I'm sending a bunch of compiler warnings for gnuplot from two
>> different compilers. The first batch comes from Sparc Solaris, the
>> second one from clang on Mac OS X 10.7 (most warnings are recent, in
>> particular those about plot2d; the last three are older).
>
> Thanks.
>
> I'm using clang also, but it hasn't given me these particular warnings.
> Almost all look like either false positives or utter trivia.
> The emf.trm declarations should be fixed, however.
> The lua.trm and wxt_gui.cpp ones I do not understand.
>
> "datafile.c", line 448: warning: syntax error: empty declaration
> EAM: True, but so what?
Usually semincolons aren't placed after function definitions. Is that
what it's complaining about?
static void auto_filetype_function(void){}; /* Just a placeholder for
auto */
> "datafile.c", line 642: warning: statement not reached
> EAM: True, and it's even commented as such in the source
I wonder why the "return NULL;" was left in? The typical error would be
"missing return", but I can't recall any C compiler complaining about
the construct whereby this function ends with the infinite loop rather
than "return". The fact it was left in with that comment makes me
suspect someone came across a compiler where "missing return" was a
problem. Odd. If there are any other similar constructs in gnuplot
code... in fact, I do see several routines which end with an infinite
loop. Since those apparently aren't causing problems, I'd say remove this:
/* NOTREACHED */
return NULL;
so that the routine is consistent with the rest.
> "set.c", line 1444: warning: argument #2 is incompatible with prototype:
> prototype: pointer to pointer to const char :
> "/opt/csw/include/iconv.h", line 83
> argument : pointer to pointer to char
>
> EAM: Feh. Ignore all warnings about "const char"
> That's not even our code - that's a system header.
Well, just changing
char degree_utf8[3] = {'\302', '\260', '\0'};
char *in = degree_utf8;
to
const char degree_utf8[3] = {'\302', '\260', '\0'};
const char *in = degree_utf8;
will get rid of the warning. Small change. The header is simply
indicating that whatever the input is, it won't be altered by the
library routine. (They probably use the same headers to compile the
library itself so the "const" enforces that rule.)
> "../term/emf.trm", line 344: warning: initializer does not fit or is
> out of range: -5
>
> EAM: This one is a true hit, however.
> It looks to me that a dozen or so of the declarations at the
> top of emf.trm are marked "unsigned" for no good reason.
> But why only complain about this one and not 10 others?
The others look to be set to valid unsigned numbers, e.g., 0, 1, FALSE,
TRUE, 0x2222, EMF_COLORS (which is 15), etc. LT_UNDEFINED (which is -5)
is the only signed value.
> "../term/hpgl.trm", line 2580: warning: statement not reached
> EAM: OK
break;
return;
Probably lose points for that one on an exam.
> "../term/lua.trm", line 469: warning: initialization type mismatch
> EAM: I have absoluately no idea what this one is about.
The definition of LUA_GP_get_boundingbox is incomplete, i.e., missing
argument definition:
static int
LUA_GP_get_boundingbox() {
should be
static int
LUA_GP_get_boundingbox(lua_State *L) {
>
> "term.c", line 2159: warning: tokens ignored at end of directive line
> EAM: ignore it
What's wrong with this line? Is it that the comment appears within the
preprocessor definitions?
# ifndef TT2$M_DECCRT3 /* VT300 not defined as of VAXC v2.4 */
or is it the $ is not a valid character so that this is really
# ifndef TT2
# define TT2
# endif
if what is after the $ is ignored?
> "wxterminal/wxt_gui.cpp", line 1669: Warning (Anachronism): Formal
> argument 1 of type extern "C" void(*)() in call to std::atexit(extern
> "C" void(*)()) is being passed void(*)().
> "wxterminal/wxt_gui.cpp", line 3831: Warning (Anachronism): Formal
> argument 2 of type extern "C" void(*)(int) in call to std::signal(int,
> extern "C" void(*)(int)) is being passed void(*)(int).
> "wxterminal/wxt_gui.cpp", line 3866: Warning (Anachronism): Formal
> argument 2 of type extern "C" void(*)(int) in call to std::signal(int,
> extern "C" void(*)(int)) is being passed void(*)(int).
> "wxterminal/wxt_gui.cpp", line 3866: Warning (Anachronism): Assigning
> extern "C" void(*)(int) to void(*)(int).
> "wxterminal/wxt_gui.cpp", line 3875: Warning (Anachronism): Formal
> argument 2 of type extern "C" void(*)(int) in call to std::signal(int,
> extern "C" void(*)(int)) is being passed void(*)(int).
> 5 Warning(s) detected.
> EAM: Is it saying that you can't pass a C routine as a C++ parameter?
> Really?
Probably. You may have to cast what is inside the argument because
these two might be different internally:
"C" void(*)()
void(*)()
the latter being a C++ function. (?) If such function resided in a
library somewhere and were called with the wrong format it would likely
cause problems. Anyway, maybe something like:
/* register call for "persist" effect and cleanup */
GP_ATEXIT(("C" void(*)())wxt_atexit);
Actually, there might be a better exit mechanism under C++ and perhaps
GP_ATEXIT() should be redefined.
> "gplt_x11.c", line 634: warning: initializer does not fit or is out of
> range: 129
> "gplt_x11.c", line 635: warning: initializer does not fit or is out of
> range: 136
> "gplt_x11.c", line 636: warning: initializer does not fit or is out of
> range: 255
> "gplt_x11.c", line 637: warning: initializer does not fit or is out of
> range: 128
> "gplt_x11.c", line 638: warning: initializer does not fit or is out of
> range: 128
> "gplt_x11.c", line 639: warning: initializer does not fit or is out of
> range: 136
> "gplt_x11.c", line 640: warning: initializer does not fit or is out of
> range: 136
> EAM: These make no sense to me.
You might have to declare stipple_pattern_bits as unsigned:
static const unsigned char stipple_pattern_bits[stipple_pattern_num][8] = {
vs.
static const char stipple_pattern_bits[stipple_pattern_num][8] = {
because some of those bit fields as hexadecimal definitons are greater
than the maximum signed char value. (Man, this compiler is going after
everything!)
> ../../original/src/plot2d.c:821:7: warning: array index of '-1'
> indexes before the beginning of the array [-Warray-bounds]
>
> COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE(cp->CRD_COLOR, v[2], cp->type,
> [snip lots of fallout]
>
> EAM: These warnings are known false positives as noted in the ChangeLog.
> The compiler is simply not smart enough to notice that the
> negative array index can never happen in practice.
Is it possible to write things to fake out the compiler? These lines
are difficult to follow for me without thinking too much, so here's a
hypothetical example:
cp = &(current_plot->points[i]);
replaced by:
cp = &(current_plot->points[i+1]) - 1;
Kludge, I know. But rewriting some of these lengthy conditionals would
be tough. (BTW, one or two letter variables in a long conditional are
challenge to debug.)
> The following patch suppresses the warnings, but I don't like it
> because it introduces unneeded code into one of the few true
> hot paths in the program. Also I'm not 100% that all C compilers
> will accept and optimize the #VARNAME syntax.
How does this fix out of range errors in a different file (plot2d.c)?
Very confusing.
> --- gnuplot/src/axis.h 2012-08-05 21:12:03.000000000 -0700
> +++ gnuplot-cvs/src/axis.h 2012-08-07 13:17:43.000000000 -0700
> @@ -539,7 +539,8 @@
> break; /* this plot is not being used for autoscaling */ \
> if (TYPE != INRANGE) \
> break; /* don't set y range if x is outrange, for example */ \
> - if (axis->linked_to_primary) { \
> + /* NB: we hope for compile-time evaluation of the strcmp */ \
> + if (strcmp(#AXIS,"COLOR_AXIS")&& axis->linked_to_primary) { \
> axis =&axis_array[AXIS - SECOND_AXES]; \
> if (axis->link_udf->at) \
> curval = eval_link_function(AXIS - SECOND_AXES, curval); \
Hope that helps some.
Dan
|
