Closed Bug 211275 Opened 22 years ago Closed 22 years ago

GetAtomHashEntry doesn't check the return value of PL_DHashTableInit

Categories

(Core :: XPCOM, defect)

x86
Windows 2000
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

()

Details

Attachments

(1 file)

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: 482 static AtomTableEntry* GetAtomHashEntry(const char* aString) 483 { 484 if ( !gAtomTable.entryCount ) 485 PL_DHashTableInit(&gAtomTable, &AtomTableOps, 0, 486 sizeof(AtomTableEntry), 2048);
this makes a few changes: 1. the uninitialized flag changed from entryCount to ops 2. the init failure => crash is fixed 3. if init succeeds and add fails we won't leak the table because of 1.
Attachment #126834 - Flags: superreview?(dbaron)
Attachment #126834 - Flags: review?(dbaron)
Comment on attachment 126834 [details] [diff] [review] switch from entryCount to ops as the uninit flag. > PL_DHashTableFinish(&gAtomTable); > gAtomTable.entryCount = 0; >+ gAtomTable.ops = nsnull; You can remove the zeroing of entryCount. There's really not much point to this since nothing null-checks the result of GetAtomHashEntry, but r=dbaron.
Attachment #126834 - Flags: superreview?(dbaron)
Attachment #126834 - Flags: superreview?
Attachment #126834 - Flags: review?(dbaron)
Attachment #126834 - Flags: review+
Comment on attachment 126834 [details] [diff] [review] switch from entryCount to ops as the uninit flag. The reason i didn't remove the line was because of: NS_COM nsrefcnt NS_GetNumberOfAtoms(void) { return gAtomTable.entryCount; }
Attachment #126834 - Flags: superreview? → superreview?(bzbarsky)
Status: NEW → ASSIGNED
Blocks: 211339
Attachment #126834 - Flags: superreview?(bzbarsky) → superreview+
checked in
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: