Menu
▾
▴
gnuplot-beta
|
From: Mojca M. <moj...@gm...> - 2012-08-28 00:01:23
|
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).
"datafile.c", line 448: warning: syntax error: empty declaration
"datafile.c", line 642: warning: statement not reached
"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
"../term/emf.trm", line 344: warning: initializer does not fit or is
out of range: -5
"../term/hpgl.trm", line 2580: warning: statement not reached
"../term/lua.trm", line 469: warning: initialization type mismatch
"term.c", line 2159: warning: tokens ignored at end of directive line
"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.
"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
"gplt_x11.c", line 1778: warning: statement not reached
../../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,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../original/src/axis.h:603:5: note: expanded from macro
'COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE'
STORE_WITH_LOG_AND_UPDATE_RANGE(STORE, VALUE, c_type_tmp, AXIS, \
^
../../original/src/axis.h:543:10: note: expanded from macro
'STORE_WITH_LOG_AND_UPDATE_RANGE'
axis = &axis_array[AXIS - SECOND_AXES]; \
^
../../original/src/axis.h:303:1: note: array 'axis_array' declared here
extern AXIS axis_array[AXIS_ARRAY_SIZE];
^
../../original/src/plot2d.c:1304:2: warning: array index of '-1'
indexes before the beginning of the array [-Warray-bounds]
STORE_WITH_LOG_AND_UPDATE_RANGE(current_plot->varcolor[i],
current_plot->varcolor[i],
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../original/src/axis.h:543:10: note: expanded from macro
'STORE_WITH_LOG_AND_UPDATE_RANGE'
axis = &axis_array[AXIS - SECOND_AXES]; \
^
../../original/src/axis.h:303:1: note: array 'axis_array' declared here
extern AXIS axis_array[AXIS_ARRAY_SIZE];
^
2 warnings generated.
../../original/src/plot3d.c:665:3: warning: array index of '-1'
indexes before the beginning of the array [-Warray-bounds]
COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE(points->CRD_COLOR, z,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../original/src/axis.h:603:5: note: expanded from macro
'COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE'
STORE_WITH_LOG_AND_UPDATE_RANGE(STORE, VALUE, c_type_tmp, AXIS, \
^
../../original/src/axis.h:543:10: note: expanded from macro
'STORE_WITH_LOG_AND_UPDATE_RANGE'
axis = &axis_array[AXIS - SECOND_AXES]; \
^
../../original/src/axis.h:303:1: note: array 'axis_array' declared here
extern AXIS axis_array[AXIS_ARRAY_SIZE];
^
../../original/src/plot3d.c:1042:4: warning: array index of '-1'
indexes before the beginning of the array [-Warray-bounds]
COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE(cp->CRD_COLOR, color, cp->type,
COLOR_AXIS, this_plot->noautoscale, NOOP, goto
come_here_if_undefined);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../original/src/axis.h:603:5: note: expanded from macro
'COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE'
STORE_WITH_LOG_AND_UPDATE_RANGE(STORE, VALUE, c_type_tmp, AXIS, \
^
../../original/src/axis.h:543:10: note: expanded from macro
'STORE_WITH_LOG_AND_UPDATE_RANGE'
axis = &axis_array[AXIS - SECOND_AXES]; \
^
../../original/src/axis.h:303:1: note: array 'axis_array' declared here
extern AXIS axis_array[AXIS_ARRAY_SIZE];
^
../../original/src/plot3d.c:1044:4: warning: array index of '-1'
indexes before the beginning of the array [-Warray-bounds]
COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE(cp->CRD_COLOR, z, cp->type,
COLOR_AXIS, this_plot->noautoscale, NOOP, goto
come_here_if_undefined);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../original/src/axis.h:603:5: note: expanded from macro
'COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE'
STORE_WITH_LOG_AND_UPDATE_RANGE(STORE, VALUE, c_type_tmp, AXIS, \
^
../../original/src/axis.h:543:10: note: expanded from macro
'STORE_WITH_LOG_AND_UPDATE_RANGE'
axis = &axis_array[AXIS - SECOND_AXES]; \
^
../../original/src/axis.h:303:1: note: array 'axis_array' declared here
extern AXIS axis_array[AXIS_ARRAY_SIZE];
^
../../original/src/plot3d.c:1207:3: warning: array index of '-1'
indexes before the beginning of the array [-Warray-bounds]
COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE(points[i].CRD_COLOR, temp,
points[i].type,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../original/src/axis.h:603:5: note: expanded from macro
'COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE'
STORE_WITH_LOG_AND_UPDATE_RANGE(STORE, VALUE, c_type_tmp, AXIS, \
^
../../original/src/axis.h:543:10: note: expanded from macro
'STORE_WITH_LOG_AND_UPDATE_RANGE'
axis = &axis_array[AXIS - SECOND_AXES]; \
^
../../original/src/axis.h:303:1: note: array 'axis_array' declared here
extern AXIS axis_array[AXIS_ARRAY_SIZE];
^
4 warnings generated.
../../original/src/util.c:518:10: warning: 'sprintf' macro redefined
# define sprintf(str,fmt,arg) \
^
/usr/include/secure/_stdio.h:49:9: note: previous definition is here
#define sprintf(str, ...) \
^
1 warning generated.
../../original/src/wxterminal/wxt_gui.cpp:2750:11: warning:
enumeration value 'command_enhanced_put_text' not handled in switch
[-Wswitch-enum]
switch ( command.command ) {
^
1 warning generated.
../../original/src/gplt_x11.c:4565:11: warning: 'XKeycodeToKeysym' is
deprecated [-Wdeprecated-declarations]
keysym = XKeycodeToKeysym(dpy, event->xkey.keycode, 0);
^
1 warning generated.
Mojca
|
|
From: Ethan M. <merritt@u.washington.edu> - 2012-08-28 04:24:07
|
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?
"datafile.c", line 642: warning: statement not reached
EAM: True, and it's even commented as such in the source
"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.
"../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?
"../term/hpgl.trm", line 2580: warning: statement not reached
EAM: OK
"../term/lua.trm", line 469: warning: initialization type mismatch
EAM: I have absoluately no idea what this one is about.
"term.c", line 2159: warning: tokens ignored at end of directive line
EAM: ignore it
"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?
"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.
../../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.
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.
--- 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); \
|
|
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
|
|
From: Mojca M. <moj...@gm...> - 2012-08-28 11:31:24
Attachments:
changes.diff
|
On Tue, Aug 28, 2012 at 8:52 AM, Daniel J Sebald wrote:
> 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 */
Yes. Removing the semicolon got rid of the warning.
>> "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.
That also solved it.
>> "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.)
Thanks. That change helped.
>> "../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.
Thanks. (But I leave it up to others to figure out which ones need a change.)
>> "../term/hpgl.trm", line 2580: warning: statement not reached
>> EAM: OK
>
> break;
> return;
>
> Probably lose points for that one on an exam.
Why is return needed here at all? (Removing it got rid of warning.)
>> "../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) {
This helped. Thank you.
>> "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
It seems so. Removing comments didn't have any influence, while
replacing $ by _ got rid of the warning. But then, this was only
diagnosis, not a valid solution.
>> "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!)
Exactly. I tried that (the only one I tried to debug myself
yesterday). But then the following complained:
XCreateBitmapFromData(dpy, plot->pixmap, stipple_pattern_bits[i],
stipple_pattern_width, stipple_pattern_height);
because XCreateBitmapFromData wants to use signed char as the 3rd
argument. In this particular case I don't really understand why
signed. (It would probably need some casting, but I'm not sure how.)
Thank you for all the diagnosis.
Oh, and of course I forgot another one:
"fit.c", line 574: warning: statement not reached
(Was it other dumber compiler that complained if return was missing?)
I'm attaching the summary of changes which only left those gplt_x11.c
warnings (I'm not sure how to properly cast) in and the complaint
about $. emf might need a closer look of course. (The bool changes
were needed since I'm unable to compile gnuplot otherwise.)
Mojca
|
|
From: sfeam (E. Merritt) <eam...@gm...> - 2012-08-28 15:47:07
|
On Monday, 27 August 2012, Daniel J Sebald wrote: > > "../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. The others are valid at initialization time, but still may be assigned one of the flag values (e.g. LT_UNDEFINED) when the code is run. If you run "grep LT_ emf.trm" you'll see many such cases. Since LT_UNDEFINED is known at compile time, I'm surprised that the compiler doesn't also complain about these other assignments. Anyhow, I've applied the trivial fixes to datafile.c emf.trm hpgl.trm The assignment of octal values to a (char) is not an error no matter what the signedness of (char), so that set of warnings is spurious and I won't change the code in gplt_x11.c or elsewhere. > # ifndef TT2$M_DECCRT3 /* VT300 not defined as of VAXC v2.4 */ > or is it the $ is not a valid character The $ is valid (and very common) in VAX/VMS identifiers. This whole section of code is inside #ifdef VMS so non-VAX compilers should not even be looking at it. |
|
From: Daniel J S. <dan...@ie...> - 2012-08-28 17:10:30
|
On 08/28/2012 10:46 AM, sfeam (Ethan Merritt) wrote: > On Monday, 27 August 2012, Daniel J Sebald wrote: >>> "../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. > > The others are valid at initialization time, but still may be > assigned one of the flag values (e.g. LT_UNDEFINED) when the > code is run. If you run "grep LT_ emf.trm" you'll see many such cases. > Since LT_UNDEFINED is known at compile time, I'm surprised that > the compiler doesn't also complain about these other assignments. True, but what is in term_api.h is a set of definitions, not an enumeration. Only in the latter does the compiler know what the valid assignment could be. An enumeration would be good, but that is too much shaking the tree this near to a release. > The assignment of octal values to a (char) is not an error no matter > what the signedness of (char), so that set of warnings is spurious and > I won't change the code in gplt_x11.c or elsewhere. Not sure. I tried searching the Internet for a reference, but there is so much static from people wondering how to print/read hexadecimal values in C that I can't find anything. The problem is that statement implies that hexadecimal values (i.e., representations) are inherently unsigned, which I'm not sure. For example, 0xFE is a valid representation in C compilers. Is -0xFE? If so, then 0xFE means 254 and -0xFE means -254. In the execution of the code, the assignment of a hexadecimal is just a byte transfer, but how the compiler interprets the abstract representation might be something different. >> # ifndef TT2$M_DECCRT3 /* VT300 not defined as of VAXC v2.4 */ >> or is it the $ is not a valid character > The $ is valid (and very common) in VAX/VMS identifiers. > This whole section of code is inside #ifdef VMS so non-VAX compilers > should not even be looking at it. Yes, from what I remember, this should be ignored. The compiler might have some internal test order wrong. Dan |
|
From: Mojca M. <moj...@gm...> - 2012-08-28 17:20:46
|
First of all, thank you for applying all the patches.
On Tue, Aug 28, 2012 at 7:10 PM, Daniel J Sebald <dan...@ie...> wrote:
> On 08/28/2012 10:46 AM, sfeam (Ethan Merritt) wrote:
>>
>> On Monday, 27 August 2012, Daniel J Sebald wrote:
>>>>
>>>>
>> The assignment of octal values to a (char) is not an error no matter
>> what the signedness of (char), so that set of warnings is spurious and
>> I won't change the code in gplt_x11.c or elsewhere.
>
>
> Not sure. I tried searching the Internet for a reference, but there is so
> much static from people wondering how to print/read hexadecimal values in C
> that I can't find anything.
>
> The problem is that statement implies that hexadecimal values (i.e.,
> representations) are inherently unsigned, which I'm not sure. For example,
> 0xFE is a valid representation in C compilers. Is -0xFE? If so, then 0xFE
> means 254 and -0xFE means -254. In the execution of the code, the
> assignment of a hexadecimal is just a byte transfer, but how the compiler
> interprets the abstract representation might be something different.
Fortran would sometime give compiler errors, not just warnings, for
trying to store FF into signed 1-byte integer.
The expression
char a = 0xff;
is (from what I believe) equivalent to
char a = 255;
and doesn't necessarily mean bitmap representation that should be
stored into a. Here's a counterexample:
float f = 0xffff;
This is definitely different from doing memcpy from 0x0000ffff from to f.
But on the other hand I agree that one doesn't need to worry about
that code so much.
>>> # ifndef TT2$M_DECCRT3 /* VT300 not defined as of VAXC v2.4 */
>>> or is it the $ is not a valid character
>>
>> The $ is valid (and very common) in VAX/VMS identifiers.
>> This whole section of code is inside #ifdef VMS so non-VAX compilers
>> should not even be looking at it.
>
> Yes, from what I remember, this should be ignored. The compiler might have
> some internal test order wrong.
I agree with both. I just wanted to reply that it was $ that was
problematic after testing. (I later discovered that I probably sent
the wrong patch. I didn't want to ask for patching this.)
What about the other two trivial patches?
--- a/src/set.c
+++ b/src/set.c
@@ -1420,10 +1420,10 @@ static void
set_degreesign(char *locale)
{
#if defined(HAVE_ICONV) && !(defined WIN32)
- char degree_utf8[3] = {'\302', '\260', '\0'};
+ const char degree_utf8[3] = {'\302', '\260', '\0'};
size_t lengthin = 3;
size_t lengthout = 8;
- char *in = degree_utf8;
+ const char *in = degree_utf8;
char *out = degree_sign;
iconv_t cd;
--- a/term/lua.trm
+++ b/term/lua.trm
@@ -168,7 +168,7 @@ static char last_error_msg[MAX_LINE_LEN+1] = "";
* the plot's bounding box
*/
static int
-LUA_GP_get_boundingbox() {
+LUA_GP_get_boundingbox(lua_State *L) {
lua_newtable (L);
lua_pushstring (L, "xleft");
lua_pushinteger(L, plot_bounds.xleft);
Mojca
|
|
From: Ethan A M. <sf...@us...> - 2012-08-28 18:29:04
|
On Tuesday, August 28, 2012 10:20:35 am Mojca Miklavec wrote:
> What about the other two trivial patches?
>
> --- a/src/set.c
> +++ b/src/set.c
> @@ -1420,10 +1420,10 @@ static void
> set_degreesign(char *locale)
> {
> #if defined(HAVE_ICONV) && !(defined WIN32)
> - char degree_utf8[3] = {'\302', '\260', '\0'};
> + const char degree_utf8[3] = {'\302', '\260', '\0'};
> size_t lengthin = 3;
> size_t lengthout = 8;
> - char *in = degree_utf8;
> + const char *in = degree_utf8;
> char *out = degree_sign;
> iconv_t cd;
That generates the following warning instead:
set.c:1444:17: warning: passing 'const char **' to parameter of type 'char **'
discards qualifiers in nested pointer types [-Wincompatible-pointer-types]
if (iconv(cd, &in, &lengthin, &out, &lengthout) == (size_t)(-1))
IMHO "const char" is impossible to get right, and warnings can safely
be ignored.
> --- a/term/lua.trm
> +++ b/term/lua.trm
> @@ -168,7 +168,7 @@ static char last_error_msg[MAX_LINE_LEN+1] = "";
> * the plot's bounding box
> */
> static int
> -LUA_GP_get_boundingbox() {
> +LUA_GP_get_boundingbox(lua_State *L) {
> lua_newtable (L);
> lua_pushstring (L, "xleft");
> lua_pushinteger(L, plot_bounds.xleft);
Wouldn't that require a corresponding change in the call site[s]?
For that matter, where _are_ the call sites - in the lua code?
Ethan
|
|
From: Mojca M. <moj...@gm...> - 2012-08-28 19:24:09
|
On Tue, Aug 28, 2012 at 8:26 PM, Ethan A Merritt wrote:
> On Tuesday, August 28, 2012 10:20:35 am Mojca Miklavec wrote:
>> What about the other two trivial patches?
>>
>> --- a/src/set.c
>> +++ b/src/set.c
>> @@ -1420,10 +1420,10 @@ static void
>> set_degreesign(char *locale)
>> {
>> #if defined(HAVE_ICONV) && !(defined WIN32)
>> - char degree_utf8[3] = {'\302', '\260', '\0'};
>> + const char degree_utf8[3] = {'\302', '\260', '\0'};
>> size_t lengthin = 3;
>> size_t lengthout = 8;
>> - char *in = degree_utf8;
>> + const char *in = degree_utf8;
>> char *out = degree_sign;
>> iconv_t cd;
>
> That generates the following warning instead:
> set.c:1444:17: warning: passing 'const char **' to parameter of type 'char **'
> discards qualifiers in nested pointer types [-Wincompatible-pointer-types]
> if (iconv(cd, &in, &lengthin, &out, &lengthout) == (size_t)(-1))
>
> IMHO "const char" is impossible to get right, and warnings can safely
> be ignored.
OK, fair enough.
>> --- a/term/lua.trm
>> +++ b/term/lua.trm
>> @@ -168,7 +168,7 @@ static char last_error_msg[MAX_LINE_LEN+1] = "";
>> * the plot's bounding box
>> */
>> static int
>> -LUA_GP_get_boundingbox() {
>> +LUA_GP_get_boundingbox(lua_State *L) {
>> lua_newtable (L);
>> lua_pushstring (L, "xleft");
>> lua_pushinteger(L, plot_bounds.xleft);
>
> Wouldn't that require a corresponding change in the call site[s]?
All other functions use lua_State *L, just this one lacks it. But I
don't know details about how it works.
> For that matter, where _are_ the call sites - in the lua code?
For example
local t = gp.get_boundingbox()
local t = gp.get_all_variables()
in gnuplot-tikz.lua
and in terminal the following mappings are registered (slightly
simplified; without #ifdefs):
static const luaL_Reg gp_methods[] = {
{"write", LUA_GP_write},
{"int_error", LUA_GP_int_error},
{"int_warn", LUA_GP_int_warn},
{"term_out", LUA_GP_term_out},
{"get_boundingbox", LUA_GP_get_boundingbox},
{"is_multiplot", LUA_GP_is_multiplot},
{"get_all_variables", LUA_GP_get_all_variables},
{"term_options", LUA_GP_term_options},
{"parse_color_name", LUA_GP_parse_color_name},
{NULL, NULL}
};
static void LUA_register_gp_fnc ()
{
LUA_register(L, "gp", gp_methods);
}
Mojca
|
