Closed
Bug 6396
Opened 26 years ago
Closed 26 years ago
I am trying to fix some compiler warnings, and I need help.
Categories
(Core :: Graphics: ImageLib, defect, P3)
Tracking
()
VERIFIED
FIXED
M10
People
(Reporter: zuperdee, Assigned: pnunn)
Details
It seems everytime I compile Mozilla here on my Red Hat 6.0 box, using EGCS
1.1.2, GLIBC 2.1, GTK 1.2.1, and the CVS tree (which I update daily and
religously), I still get (literally) thousands of compiler warnings. I am
trying to fix some of them... But I need some help.
Now, this particular set of warnings have to do with:
/modules/libimg/src/scale.cpp
I am assuming this is part of the ImageLib module... Correct me if I'm
mistaken. The warnings I get are:
scale.cpp: In function `void il_emit_row(struct il_container *, uint8 *, uint8
*, int, int, int, int, enum il_draw_mode, int)':
scale.cpp:718: warning: comparison between signed and unsigned
scale.cpp:744: warning: comparison between signed and unsigned
scale.cpp:750: warning: comparison between signed and unsigned
scale.cpp:771: warning: comparison between signed and unsigned
scale.cpp:777: warning: comparison between signed and unsigned
scale.cpp: At top level:
scale.cpp:55: warning: `class nsVoidArray * gTimeouts' defined but not used
I then went first of all to line 718, where this is the bit of code in question:
PR_ASSERT(row >= 0);
if(row >= src_header->height) {
ILTRACE(2,("il: ignoring extra row (%d)", row));
return;
}
Obviously, with row being an int, and src_header->height probably being
unsigned, this results in conflict. The easiest way to fix this would probably
be to do a type-cast on the variable "row," but I have a question to ask about
this first, just to make sure I'm being thorough: Since the statement
PR_ASSERT(row >= 0); is here, I am assuming this represents a sanity check to
make sure row is not <= 0. But if it is correct that this should always be the
case, then why not make the variable "row" unsigned, so it will match the type
of the "height" variable, and no type cast is even needed?? This might make the
code even more clear and logical. The same solution could be applied to the
other warnings as well--that is, make ALL of these unsigned, so they will match
"height." The other thing that makes me think this is the best solution is that
I cannot think of any good reason why these variables ever would be negative
anyway--again, somebody correct me if I'm wrong.
As to the warning on line 55, the simple solution here would be to simply delete
that line altogether, unless somebody is anticipating future use of it. But
even then, if that is the case, it should be commented out, with an explanation
given as to why it is there.
Since I am just a beginner, having just completed a couple of introductory C/C++
programming courses, I would appreciate it if people could give me feedback on
this report; I want to know if I am on the right track. I believe code should
be clear and logical.
While noting warnings is good, other bugs have some
priority. Try setting your warning level to trim your list.
-pn
Reporter | ||
Updated•26 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 2•26 years ago
|
||
The number of build warnings is slowly decreasing... And the particular
warnings I noted in this bug no longer exist... This bug is no longer of
interest to me. I therefore would like to mark it fixed so as to get it off
your radar. Somebody want to verify my resolution?
Updated•26 years ago
|
Status: RESOLVED → VERIFIED
Comment 3•26 years ago
|
||
Rubber-stamped verified.
You need to log in
before you can comment on or make changes to this bug.
Description
•