Closed
Bug 185009
Opened 22 years ago
Closed 20 years ago
Uninitialize variable potentially used.
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bratell, Unassigned)
Details
Attachments
(2 files)
678 bytes,
patch
|
axel
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
1.04 KB,
patch
|
peterv
:
superreview+
|
Details | Diff | Splinter Review |
In nsGenericHTMLElement::CopyInnerTo the code at the end looks like: PRInt32 id; if (mDocument) { mDocument->GetAndIncrementContentID(&id); } aDst->SetContentID(id); // <-- id will be uninitialized if mDocument is null. It should probably be: if (mDocument) { PRInt32 id; mDocument->GetAndIncrementContentID(&id); aDst->SetContentID(id); }
Comment 2•21 years ago
|
||
fix for bug
Comment 3•21 years ago
|
||
Comment on attachment 121637 [details] [diff] [review] fixes bug Can I get R/SR/etc on this?
Attachment #121637 -
Flags: superreview?
Attachment #121637 -
Flags: review?
Comment 4•21 years ago
|
||
Comment on attachment 121637 [details] [diff] [review] fixes bug sounds like a descent idea in general, but I'm not sure whether the DOM slots are in a sane state without SetContentID. So maybe just doing a PRInt32 id = 0; may be the better fix. peterv?
Attachment #121637 -
Flags: superreview?(peterv)
Attachment #121637 -
Flags: superreview?
Attachment #121637 -
Flags: review?
Attachment #121637 -
Flags: review+
Updated•21 years ago
|
Attachment #121637 -
Flags: superreview?(peterv) → superreview+
Comment 5•21 years ago
|
||
if no document, should we be asserting, and calling SetContentID(-1)? and / or returning NS_ERROR_UNEXPECTED? peterv, what do you think?
Comment 6•21 years ago
|
||
No need to assert just because there's no document. This patch gets the document from the elements nodeinfo, and as long as the document where the element came from still exists, we'll get it, even if the element is no longer part of the document. If we can't get a document, we'll set the content id to -1, which IMO is what we should do. This will cause slots to be allocated, so it's a bit expensive, but as long as we get the document from the nodeinfo, that case will be so rare that I don't see how that could matter.
Comment 7•21 years ago
|
||
Comment on attachment 121840 [details] [diff] [review] Default to -1, and get the document from mNodeInfo I guess so, though I don't like passing negative numbers to a function that takes a PRUint32. Seth: it is not an error to not have a mDocument.
Attachment #121840 -
Flags: superreview+
Comment 8•21 years ago
|
||
Um, right. s/-1/PR_UINT32_MAX/ on that patch.
Comment 9•20 years ago
|
||
Fixed as part of bug 215981.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Component: DOM: HTML → DOM: Core & HTML
QA Contact: stummala → general
You need to log in
before you can comment on or make changes to this bug.
Description
•