211284 - SetOrRemoveObject doesn't check the return value of PL_DHashTableInit

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[timeless]

This is a code inspection bug.

QA:
To verify that this bug is fixed, make sure that all callers in of 
PL_DHashTableInit in the indicated class/file check and handle a failure return 
result.
To verify that this bug is invalid, simply verify (e.g. with lxr) that 
PL_DHashTableInit is no longer used by the indicated class/file.

PL_DHashTableInit returns a PRBool indicating whether it succeeded. 
PL_DHashTableInit *can* fail. Code can not assume that table->ops or table-
>data will be null (in fact it probably will not be), although in all 
likelyhood table->entryStore will be null it probably isn't safe to assume this.

Code at time of bug filing:


 297 static nsresult
 298 SetOrRemoveObject(PLDHashTable& table, nsISupports* aKey, nsISupports* 
aValue)
 299 {
 300   // lazily create the table, but don't create it just to remove a
 301   // non-existent element!
 302   if (!table.ops && aValue)
 303     PL_DHashTableInit(&table, &ObjectTableOps, nsnull,
 304                       sizeof(ObjectEntry), 16);
 305
 306   if (aValue)
 307     return AddObjectEntry(table, aKey, aValue);
 308
 309   // no value, so remove the key from the table
 310   if (table.ops)
 311     RemoveObjectEntry(table, aKey);
 312   return NS_OK;
 313 }

Comment 1

[timeless]

Attachment: check

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 126840 [details] [diff] [review]
check

sr=dbaron, but why not move the whole thing into the |if (aValue)| below?

Comment 3

[timeless]

will do

Comment 4

[Alec Flett]

Comment on attachment 126840 [details] [diff] [review]
check

sr=alecf

Comment 5

[timeless]

checked in

Comment 6

[timeless]

checked in