Closed Bug 562661 Opened 10 years ago Closed 10 years ago

[@ nsNavHistory::RowToResult] mishandles *aResult

Categories

(Toolkit :: Places, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, crash)

Crash Data

Attachments

(1 file, 1 obsolete file)

6781 	nsNavHistory::RowToResult(mozIStorageStatement* aRow,
6785 	  *aResult = nsnull;
6786 	  NS_ASSERTION(aRow && aOptions && aResult, "Null pointer in RowToResult");

6845 	    if (*aResult && (*aResult)->IsFolder() &&
6846 	         aOptions->ResultType() != 
6847 	           nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY)

6851 	    if (aOptions->ResultType() == 
6852 	      (*aResult)->mDateAdded =
Attached patch rewrite it (obsolete) — Splinter Review
1. this drops the null check after crashing
2. the if pair is really using the same condition for opposite cases, with the first using a second condition, this is rewritten to handle the second case first.
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #442407 - Flags: review?(dietrich)
why not moving the NS_ASSERTION above crashing instead?

    rv = QueryRowToResult(itemId, url, title, accessCount, time, favicon, aResult);

uh, where is error checking? sigh :(

+    if (*aResult) {

we really should have *aResult here, it should probably just be
NS_ENSURE_STATE(*aResult);
Attached patch per makSplinter Review
Attachment #442407 - Attachment is obsolete: true
Attachment #442576 - Flags: review?(mak77)
Attachment #442407 - Flags: review?(dietrich)
Comment on attachment 442576 [details] [diff] [review]
per mak

looks good!
Attachment #442576 - Flags: review?(mak77) → review+
http://hg.mozilla.org/mozilla-central/rev/bfdd5c8dbef0
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Crash Signature: [@ nsNavHistory::RowToResult]
You need to log in before you can comment on or make changes to this bug.