Closed Bug 333356 Opened 14 years ago Closed 14 years ago

cellmap dead code & 0 deref

Categories

(Core :: Layout: Tables, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bernd_mozilla, Assigned: bernd_mozilla)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, fixed1.8.0.5, fixed1.8.1, Whiteboard: [need testcase])

Attachments

(1 file)

fix for two coverity warnings
Attached patch patchSplinter Review
Attachment #217771 - Flags: superreview?(bzbarsky)
Attachment #217771 - Flags: review?(bzbarsky)
Comment on attachment 217771 [details] [diff] [review]
patch

>Index: nsCellMap.cpp

>+    // at the right edge of the table as we checked the corner before

Can we add an assert to that effect?  That is, I assume we'd assert !aIsBottomRight here?

r+sr=bzbarsky with that.
Attachment #217771 - Flags: superreview?(bzbarsky)
Attachment #217771 - Flags: superreview+
Attachment #217771 - Flags: review?(bzbarsky)
Attachment #217771 - Flags: review+
Keywords: coverity
fix checked in
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
roc, bz: is this wanted/appropriate for 1.8.0.x also?
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
Flags: blocking1.8.0.5? → blocking1.8.0.5+
Comment on attachment 217771 [details] [diff] [review]
patch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #217771 - Flags: approval1.8.0.5+
Attachment #217771 - Flags: approval-branch-1.8.1?(roc)
Attachment #217771 - Flags: approval-branch-1.8.1+
fix checked in into branches
>Can we add an assert to that effect? That is, I assume we'd assert
>!aIsBottomRight here?
> r+sr=bzbarsky with that.

The patches checked in do not have the assert.

>The patches checked in do not have the assert.

and thats intentional!!! Nobody is debugging layout on a branch, the trunk where people debug has the assert see http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsCellMap.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/tables&command=DIFF_FRAMESET&rev1=3.101&rev2=3.102
 and yes, I did learn my lesson from bug 315210. 
But please go and put in the assert if you can handle the consequences I will certainly not do it.
Any easy way to verify this fix?  Any testcases or steps associated with the Coverity warnings mentioned?
Whiteboard: [need testcase]
just get a coverity account and ask it if it complains about the relevant files. for kicks, find a run from before this bug was filed and verify that coverity indeed lists it as a complaint.
Flags: blocking1.8.1?
You need to log in before you can comment on or make changes to this bug.