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)

x86
Linux
defect

Tracking

()

VERIFIED FIXED

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.
Status: NEW → ASSIGNED
Target Milestone: M7
While noting warnings is good, other bugs have some priority. Try setting your warning level to trim your list. -pn
Target Milestone: M7 → M10
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
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?
Status: RESOLVED → VERIFIED
Rubber-stamped verified.
You need to log in before you can comment on or make changes to this bug.