Closed
Bug 52285
Opened 24 years ago
Closed 21 years ago
Kill (unused & redeclaration) compiler warnings in layout/xul
Categories
(Core :: Layout, defect, P3)
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).
Reporter | ||
Comment 1•24 years ago
|
||
Reporter | ||
Updated•24 years ago
|
Reporter | ||
Comment 3•24 years ago
|
||
In testing this patch breaks the UI layout. I may have missed a side-effect
somewhere. Stay tuned for a second, tested patch.
Reporter | ||
Comment 4•24 years ago
|
||
Slip of the keyboard in the nsSprocketLayout.cpp changes. The second patch is
attached and smoketested on Linux. Sorry for the noise.
Reporter | ||
Comment 5•24 years ago
|
||
Comment 6•24 years ago
|
||
Thanks for the patch, futuring to get off radar, but hopefully EVaughan
will still have a chance to review this.
Target Milestone: --- → Future
Updated•23 years ago
|
Attachment #14496 -
Attachment is obsolete: true
Comment 7•22 years ago
|
||
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
Comment 8•22 years ago
|
||
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
Comment 9•22 years ago
|
||
CCing people blamed for some of the warnings my patch fixes.
Can somebody please review my patch? Thanks a lot!
Target Milestone: Future → ---
Comment 10•22 years ago
|
||
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...
Comment 11•22 years ago
|
||
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
Comment 12•22 years ago
|
||
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 13•22 years ago
|
||
Comment on attachment 105782 [details] [diff] [review]
Clean up a bunch of warnings, v3
[Checked in: Comment 15]
sr=dbaron
Attachment #105782 -
Flags: superreview+
Updated•22 years ago
|
Attachment #105782 -
Flags: review?
Attachment #105782 -
Flags: review? → review+
Comment 14•22 years ago
|
||
Can someboby please check this in (I do not have CVS access)? Thanks a lot!
Comment 15•22 years ago
|
||
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 16•22 years ago
|
||
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
Updated•22 years ago
|
Blocks: buildwarning
Updated•21 years ago
|
Attachment #105782 -
Attachment description: Clean up a bunch of warnings, v3 → Clean up a bunch of warnings, v3
[Checked in: Comment 15]
Comment 17•21 years ago
|
||
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.
Description
•