562661 - [@ nsNavHistory::RowToResult] mishandles *aResult

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[timeless]

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 =

Comment 1

[timeless]

Attachment: rewrite it

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.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

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);

Comment 3

[timeless]

Attachment: per mak

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 442576 [details] [diff] [review]
per mak

looks good!

Comment 5

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/bfdd5c8dbef0