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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: timeless)
References
()
Details
Attachments
(1 file)
3.50 KB,
patch
|
dbaron
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
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)
![]() |
||
Updated•22 years ago
|
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.
Description
•