Closed Bug 185009 Opened 22 years ago Closed 20 years ago

Uninitialize variable potentially used.

Categories

(Core :: DOM: Core & HTML, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bratell, Unassigned)

Details

Attachments

(2 files)

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);
  }
Mass-reassigning bugs.
Assignee: jst → dom_bugs
Attached patch fixes bugSplinter Review
fix for bug
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 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+
Attachment #121637 - Flags: superreview?(peterv) → superreview+
if no document, should we be asserting, and calling

SetContentID(-1)?

and / or returning NS_ERROR_UNEXPECTED?

peterv, what do you think?
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 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+
Um, right. s/-1/PR_UINT32_MAX/ on that patch.
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.

Attachment

General

Created:
Updated:
Size: