Closed Bug 52285 Opened 20 years ago Closed 17 years ago
Kill (unused & redeclaration) compiler warnings in layout/xul
Attached is a patch which fixes the legitimate compiler warnings in mozilla/layout/xul/base/src. Per the advice from bug 52234, I have not tried to suppress any warnings that are due to GCC's lack of understanding. The patch removes 7 completely unused variables and two useless redeclarations of local variables. CC those of you who were blamed in tinderbox (and brendan because he is a smart and helpful guy).
Most of these look like Eric's. /be
Assignee: clayton → evaughan
In testing this patch breaks the UI layout. I may have missed a side-effect somewhere. Stay tuned for a second, tested patch.
Slip of the keyboard in the nsSprocketLayout.cpp changes. The second patch is attached and smoketested on Linux. Sorry for the noise.
Thanks for the patch, futuring to get off radar, but hopefully EVaughan will still have a chance to review this.
Target Milestone: --- → Future
Is this bug completely forgotten? It has an almost two year old patch attached to it... Bug 132141 is also for warnings in layout/xul
Depends on: 132141
In a few cases I chose to throw away the unused rv wariable in a function that always returns NS_OK. Would it be better to make it return rv instead?
Attachment #14499 - Attachment is obsolete: true
CCing people blamed for some of the warnings my patch fixes. Can somebody please review my patch? Thanks a lot!
Comment on attachment 84427 [details] [diff] [review] Clean up a bunch of warnings. >Index: layout/xul/base/src/nsBox.cpp >- sprintf(addr, "[@%p] ", frame); >+ sprintf(addr, "[@%p] ", (void *) frame); Prefer NS_STATIC_CAST(void*, frame) to old-style casts. >Index: layout/xul/base/src/nsImageBoxFrame.cpp >- nsresult rv = nsLeafBoxFrame::AttributeChanged(aPresContext, aChild, aNameSpaceID, aAttribute, aModType, aHint); >+ nsLeafBoxFrame::AttributeChanged(aPresContext, aChild, aNameSpaceID, aAttribute, aModType, aHint); How about checking the return value and returning, i.e., if (NS_FAILED(rv)) return rv; >Index: layout/xul/base/src/nsMenuPopupFrame.cpp Same here. >Index: layout/xul/base/src/nsScrollBoxFrame.cpp And here? Then again, lots of layout code just ignores nsresult return values, so they don't really mean much and it's probably not that bad to ignore a few more...
Incorporated David's suggestions - use NS_STATIC_CAST; do something reasonable with previously unused rv's - in tow cases out of three I now return the rv instead of discarding it, but in the third case just discarding it seems like a natural thing to do.
Attachment #84427 - Attachment is obsolete: true
Updating for bit rot. Previous patch was sitting around with no activity for almost half a year. Hopefully this one would be luckier. Anybody care to review? Thanks!
Attachment #84456 - Attachment is obsolete: true
Comment on attachment 105782 [details] [diff] [review] Clean up a bunch of warnings, v3 [Checked in: Comment 15] sr=dbaron
Attachment #105782 - Flags: superreview+
Can someboby please check this in (I do not have CVS access)? Thanks a lot!
this is now checked in 11/18/2002 21:06 timeless%mozdev.org mozilla/ layout/ xul/ base/ src/ grid/ nsGridRowLeafFrame.cpp 1.4 0/1 Bug 52285 Kill compiler warnings in layout/xul patch by firstname.lastname@example.org r=timeless sr=dbaron (and other files)
Comment on attachment 105782 [details] [diff] [review] Clean up a bunch of warnings, v3 [Checked in: Comment 15] This was checked in and fixed 12 or so warnings. There are still a number of meaningful warnings in layout/xul though, so we should keep the bug open.
Attachment #105782 - Attachment is obsolete: true
Attachment #105782 - Attachment description: Clean up a bunch of warnings, v3 → Clean up a bunch of warnings, v3 [Checked in: Comment 15]
Updating: *(S) New -> R.Fixed, in an attempt to sort the many "warning" bugs.
Status: NEW → RESOLVED
Closed: 17 years ago
No longer depends on: 132141
Resolution: --- → FIXED
Summary: Kill compiler warnings in layout/xul → Kill (unused & redeclaration) compiler warnings in layout/xul
You need to log in before you can comment on or make changes to this bug.