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

RESOLVED FIXED

Status

()

Core
RDF
--
minor
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

15 years ago
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);
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

15 years ago
Created attachment 126830 [details] [diff] [review]
proposed changes
(Assignee)

Updated

15 years ago
Attachment #126830 - Flags: superreview?(dbaron)
Attachment #126830 - Flags: review?(suresh)
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).
Attachment #126830 - Flags: superreview?(dbaron) → superreview+

Comment 3

15 years ago
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
Attachment #126830 - Flags: review?(suresh) → review+
(Assignee)

Comment 4

15 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.