211270 - InMemoryDataSource::Init doesn't check the return value of PL_DHashTableInit

Status: RESOLVED FIXED

(Core Graveyard :: RDF, 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:

 896 nsresult
 897 InMemoryDataSource::Init()
 898 {
 899     PL_DHashTableInit(&mForwardArcs,
 900                       PL_DHashGetStubOps(),
 901                       nsnull,
 902                       sizeof(Entry),
 903                       PL_DHASH_MIN_SIZE);
 904
 905     PL_DHashTableInit(&mReverseArcs,
 906                       PL_DHashGetStubOps(),
 907                       nsnull,
 908                       sizeof(Entry),
 909                       PL_DHASH_MIN_SIZE);

Comment 1

[timeless]

Attachment: proposed changes

Comment 2

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

Comment on attachment 126830 [details] [diff] [review]
proposed changes

sr=dbaron, although maybe it would better to set both ops pointers to null in
the constructor to parallel the way mLock is handled (and then set only the
current one to null for each failure case in Init).

Comment 3

[Chase Tingley]

Comment on attachment 126830 [details] [diff] [review]
proposed changes

I second dbaron's suggestion.  Also, consider moving the comment about only
having to enumerate once inside the if braces.
r=tingley

Comment 4

[timeless]

checked in