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

RESOLVED FIXED

Status

()

defect
P3
normal
RESOLVED FIXED
19 years ago
16 years ago

People

(Reporter: jwbaker, Assigned: eric)

Tracking

(Blocks 1 bug)

Trunk
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(5 obsolete attachments)

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).
Posted patch Patch (obsolete) — 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.
Posted patch Corrected Patch (obsolete) — 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
Posted patch Clean up a bunch of warnings. (obsolete) — 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...
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: 16 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.