Closed Bug 52285 Opened 24 years ago Closed 21 years ago

Kill (unused & redeclaration) compiler warnings in layout/xul

Categories

(Core :: Layout, defect, P3)

x86
Linux
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: jwbaker, Assigned: eric)

References

(Blocks 1 open bug, )

Details

Attachments

(5 obsolete files)

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).
Attached patch Patch (obsolete) (deleted) — Splinter Review
Keywords: patch, review
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.
Attached patch Corrected Patch (obsolete) (deleted) — Splinter Review
Thanks for the patch, futuring to get off radar, but hopefully EVaughan will still have a chance to review this.
Target Milestone: --- → Future
Attachment #14496 - Attachment is obsolete: true
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
Attached patch Clean up a bunch of warnings. (obsolete) (deleted) — Splinter Review
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...
Attached patch Clean up a bunch of warnings, v2 (obsolete) (deleted) — Splinter Review
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+
Attachment #105782 - Flags: review?
Attachment #105782 - Flags: review? → review+
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 mozilla-bugs@nogin.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
Blocks: buildwarning
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: 21 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.

Attachment

General

Creator:
Created:
Updated:
Size: