cellmap dead code & 0 deref

RESOLVED FIXED

Status

()

Core
Layout: Tables
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: Bernd, Assigned: Bernd)

Tracking

({coverity, fixed1.8.0.5, fixed1.8.1})

Trunk
x86
Windows XP
coverity, fixed1.8.0.5, fixed1.8.1
Points:
---
Bug Flags:
blocking1.8.0.5 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [need testcase])

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
fix for two coverity warnings
(Assignee)

Comment 1

12 years ago
Created attachment 217771 [details] [diff] [review]
patch
Attachment #217771 - Flags: superreview?(bzbarsky)
Attachment #217771 - Flags: review?(bzbarsky)

Comment 2

12 years ago
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+

Updated

12 years ago
Keywords: coverity
(Assignee)

Comment 3

11 years ago
fix checked in
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

11 years ago
Comment on attachment 217771 [details] [diff] [review]
patch

I checked it with boris comments 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
Attachment #217771 - Flags: approval-branch-1.8.1?(roc)
roc, bz: is this wanted/appropriate for 1.8.0.x also?
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?

Updated

11 years ago
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+
(Assignee)

Comment 7

11 years ago
fix checked in into branches
Keywords: fixed1.8.0.5, fixed1.8.1

Comment 8

11 years ago
>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.

(Assignee)

Comment 9

11 years ago
>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.

Comment 10

11 years ago
Any easy way to verify this fix?  Any testcases or steps associated with the Coverity warnings mentioned?
Whiteboard: [need testcase]

Comment 11

11 years ago
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.