Closed Bug 586603 Opened 9 years ago Closed 9 years ago

HTMLContentSink::BeginContext should null check mCurrentContext before creating sc

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, memory-leak)

Attachments

(1 file, 1 obsolete file)

1760 HTMLContentSink::BeginContext(PRInt32 aPosition)

too early:
1764   // Create new context
1765   SinkContext* sc = new SinkContext(this);
1766   if (!sc) {
1767     return NS_ERROR_OUT_OF_MEMORY;
1768   }

1770   if (!mCurrentContext) {
1771     NS_ERROR("Nonexistent context");
leaked:
1773     return NS_ERROR_FAILURE;
1774   }
1775 
1776   // Flush everything in the current context so that we don't have
1777   // to worry about insertions resulting in inconsistent frame creation.
1778   mCurrentContext->FlushTags();
1779 
1780   // Sanity check.
1781   if (mCurrentContext->mStackPos <= aPosition) {
1782     NS_ERROR("Out of bounds position");
leaked:
1783     return NS_ERROR_FAILURE;
1784   }
1785 
1786   PRInt32 insertionPoint = -1;
1787   nsHTMLTag nodeType      = mCurrentContext->mStack[aPosition].mType;
1788   nsGenericHTMLElement* content = mCurrentContext->mStack[aPosition].mContent;
1789 
1790   // If the content under which the new context is created
1791   // has a child on the stack, the insertion point is
1792   // before the last child.
1793   if (aPosition < (mCurrentContext->mStackPos - 1)) {
1794     insertionPoint = content->GetChildCount() - 1;
1795   }

just right:
1797   sc->Begin(nodeType,
1798             content,
1799             mCurrentContext->mStack[aPosition].mNumFlushed,
1800             insertionPoint);
1801   NS_ADDREF(sc->mSink);
1802 
1803   mContextStack.AppendElement(mCurrentContext);
1804   mCurrentContext = sc;
1805   return NS_OK;
1806 }
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #465175 - Flags: review?(Olli.Pettay)
Might as well remove the
1766   if (!sc) {
1767     return NS_ERROR_OUT_OF_MEMORY;
1768   }
as new is infallible...
Comment on attachment 465175 [details] [diff] [review]
patch

Actually, there is no need for null check.
new is infallible.
Attachment #465175 - Flags: review?(Olli.Pettay) → review-
Attached patch less codeSplinter Review
Attachment #465175 - Attachment is obsolete: true
Attachment #466570 - Flags: review?(Olli.Pettay)
Attachment #466570 - Flags: review?(Olli.Pettay) → review+
Attachment #466570 - Flags: approval2.0?
Comment on attachment 466570 [details] [diff] [review]
less code

Mass minusing patch approval that don't have high return. Please renominate if this is more important for 2.0 than it appears.
Attachment #466570 - Flags: approval2.0? → approval2.0-
http://hg.mozilla.org/mozilla-central/rev/d7cbfae806d9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla2.2
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.