211275 - GetAtomHashEntry doesn't check the return value of PL_DHashTableInit

Status: RESOLVED FIXED

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

 482 static AtomTableEntry* GetAtomHashEntry(const char* aString)
 483 {
 484   if ( !gAtomTable.entryCount )
 485     PL_DHashTableInit(&gAtomTable, &AtomTableOps, 0,
 486                       sizeof(AtomTableEntry), 2048);

Comment 1

[timeless]

Attachment: switch from entryCount to ops as the uninit flag.

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.

Comment 2

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

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.

Comment 3

[timeless]

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

Comment 4

[timeless]

checked in