Skip to content

Commit

Permalink
Bug 1159972 - Remove the fallible version of PL_DHashTableInit(). r=f…
Browse files Browse the repository at this point in the history
…roydnj.

It's no longer needed now that entry storage isn't allocated there. (The other
possible causes of failures in that function are less interesting and simply
crashing is a reasonable thing to do for them.)

This also makes PL_DNewHashTable() infallible, so I removed some
now-unnecessary checks of its result.
  • Loading branch information
nnethercote committed Apr 29, 2015
1 parent 73477cf commit e8e6640
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 173 deletions.
3 changes: 0 additions & 3 deletions dom/base/nsDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4047,9 +4047,6 @@ nsDocument::SetSubDocumentFor(Element* aElement, nsIDocument* aSubDoc)
};

mSubDocuments = PL_NewDHashTable(&hash_table_ops, sizeof(SubDocMapEntry));
if (!mSubDocuments) {
return NS_ERROR_OUT_OF_MEMORY;
}
}

// Add a mapping to the hash table
Expand Down
22 changes: 8 additions & 14 deletions dom/base/nsScriptNameSpaceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,21 +323,15 @@ nsScriptNameSpaceManager::Init()
GlobalNameHashInitEntry
};

mIsInitialized = PL_DHashTableInit(&mGlobalNames, &hash_table_ops,
sizeof(GlobalNameMapEntry),
fallible,
GLOBALNAME_HASHTABLE_INITIAL_LENGTH);
NS_ENSURE_TRUE(mIsInitialized, NS_ERROR_OUT_OF_MEMORY);

mIsInitialized = PL_DHashTableInit(&mNavigatorNames, &hash_table_ops,
sizeof(GlobalNameMapEntry),
fallible,
GLOBALNAME_HASHTABLE_INITIAL_LENGTH);
if (!mIsInitialized) {
PL_DHashTableFinish(&mGlobalNames);
PL_DHashTableInit(&mGlobalNames, &hash_table_ops,
sizeof(GlobalNameMapEntry),
GLOBALNAME_HASHTABLE_INITIAL_LENGTH);

return NS_ERROR_OUT_OF_MEMORY;
}
PL_DHashTableInit(&mNavigatorNames, &hash_table_ops,
sizeof(GlobalNameMapEntry),
GLOBALNAME_HASHTABLE_INITIAL_LENGTH);

mIsInitialized = true;

RegisterWeakMemoryReporter(this);

Expand Down
5 changes: 0 additions & 5 deletions dom/xul/XULDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -769,11 +769,6 @@ XULDocument::AddBroadcastListenerFor(Element& aBroadcaster, Element& aListener,

if (! mBroadcasterMap) {
mBroadcasterMap = PL_NewDHashTable(&gOps, sizeof(BroadcasterMapEntry));

if (! mBroadcasterMap) {
aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
return;
}
}

BroadcasterMapEntry* entry =
Expand Down
2 changes: 0 additions & 2 deletions layout/style/nsRuleNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1617,8 +1617,6 @@ nsRuleNode::ConvertChildrenToHash(int32_t aNumKids)
PLDHashTable *hash = PL_NewDHashTable(&ChildrenHashOps,
sizeof(ChildrenHashEntry),
aNumKids);
if (!hash)
return;
for (nsRuleNode* curr = ChildrenList(); curr; curr = curr->mNextSibling) {
// This will never fail because of the initial size we gave the table.
ChildrenHashEntry *entry = static_cast<ChildrenHashEntry*>(
Expand Down
7 changes: 2 additions & 5 deletions modules/libpref/prefapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,8 @@ static nsresult pref_HashPref(const char *key, PrefValue value, PrefType type, u
nsresult PREF_Init()
{
if (!gHashTable.IsInitialized()) {
if (!PL_DHashTableInit(&gHashTable, &pref_HashTableOps,
sizeof(PrefHashEntry), fallible,
PREF_HASHTABLE_INITIAL_LENGTH)) {
return NS_ERROR_OUT_OF_MEMORY;
}
PL_DHashTableInit(&gHashTable, &pref_HashTableOps,
sizeof(PrefHashEntry), PREF_HASHTABLE_INITIAL_LENGTH);

PL_INIT_ARENA_POOL(&gPrefNameArena, "PrefNameArena",
PREFNAME_ARENA_SIZE);
Expand Down
11 changes: 3 additions & 8 deletions netwerk/cache/nsCacheEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,14 +402,9 @@ nsCacheEntryHashTable::~nsCacheEntryHashTable()
nsresult
nsCacheEntryHashTable::Init()
{
nsresult rv = NS_OK;
initialized = PL_DHashTableInit(&table, &ops,
sizeof(nsCacheEntryHashTableEntry),
fallible, 256);
if (!initialized) rv = NS_ERROR_OUT_OF_MEMORY;
return rv;
PL_DHashTableInit(&table, &ops, sizeof(nsCacheEntryHashTableEntry), 256);
initialized = true;
return NS_OK;
}
void
Expand Down
6 changes: 2 additions & 4 deletions netwerk/protocol/http/nsHttp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,8 @@ nsHttp::CreateAtomTable()
// The initial length for this table is a value greater than the number of
// known atoms (NUM_HTTP_ATOMS) because we expect to encounter a few random
// headers right off the bat.
if (!PL_DHashTableInit(&sAtomTable, &ops, sizeof(PLDHashEntryStub),
fallible, NUM_HTTP_ATOMS + 10)) {
return NS_ERROR_OUT_OF_MEMORY;
}
PL_DHashTableInit(&sAtomTable, &ops, sizeof(PLDHashEntryStub),
NUM_HTTP_ATOMS + 10);

// fill the table with our known atoms
const char *const atoms[] = {
Expand Down
15 changes: 4 additions & 11 deletions parser/htmlparser/nsHTMLEntities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,10 @@ nsresult
nsHTMLEntities::AddRefTable(void)
{
if (!gTableRefCnt) {
if (!PL_DHashTableInit(&gEntityToUnicode, &EntityToUnicodeOps,
sizeof(EntityNodeEntry),
fallible, NS_HTML_ENTITY_COUNT)) {
return NS_ERROR_OUT_OF_MEMORY;
}
if (!PL_DHashTableInit(&gUnicodeToEntity, &UnicodeToEntityOps,
sizeof(EntityNodeEntry),
fallible, NS_HTML_ENTITY_COUNT)) {
PL_DHashTableFinish(&gEntityToUnicode);
return NS_ERROR_OUT_OF_MEMORY;
}
PL_DHashTableInit(&gEntityToUnicode, &EntityToUnicodeOps,
sizeof(EntityNodeEntry), NS_HTML_ENTITY_COUNT);
PL_DHashTableInit(&gUnicodeToEntity, &UnicodeToEntityOps,
sizeof(EntityNodeEntry), NS_HTML_ENTITY_COUNT);
for (const EntityNode *node = gEntityArray,
*node_end = ArrayEnd(gEntityArray);
node < node_end; ++node) {
Expand Down
6 changes: 2 additions & 4 deletions security/manager/ssl/src/nsCertTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,8 @@ void nsCertTree::ClearCompareHash()
nsresult nsCertTree::InitCompareHash()
{
ClearCompareHash();
if (!PL_DHashTableInit(&mCompareCache, &gMapOps,
sizeof(CompareCacheHashEntryPtr), fallible, 64)) {
return NS_ERROR_OUT_OF_MEMORY;
}
PL_DHashTableInit(&mCompareCache, &gMapOps,
sizeof(CompareCacheHashEntryPtr), 64);
return NS_OK;
}

Expand Down
7 changes: 2 additions & 5 deletions xpcom/ds/nsStaticNameTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,8 @@ nsStaticCaseInsensitiveNameTable::Init(const char* const aNames[],
return false;
}

if (!PL_DHashTableInit(&mNameTable, &nametable_CaseInsensitiveHashTableOps,
sizeof(NameTableEntry), fallible,
aLength)) {
return false;
}
PL_DHashTableInit(&mNameTable, &nametable_CaseInsensitiveHashTableOps,
sizeof(NameTableEntry), aLength);

for (int32_t index = 0; index < aLength; ++index) {
const char* raw = aNames[index];
Expand Down
38 changes: 7 additions & 31 deletions xpcom/glue/pldhash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,7 @@ PL_NewDHashTable(const PLDHashTableOps* aOps, uint32_t aEntrySize,
uint32_t aLength)
{
PLDHashTable* table = new PLDHashTable();
if (!PL_DHashTableInit(table, aOps, aEntrySize, fallible, aLength)) {
delete table;
return nullptr;
}
PL_DHashTableInit(table, aOps, aEntrySize, aLength);
return table;
}

Expand Down Expand Up @@ -217,9 +214,9 @@ MinCapacity(uint32_t aLength)
return (aLength * 4 + (3 - 1)) / 3; // == ceil(aLength * 4 / 3)
}

MOZ_ALWAYS_INLINE bool
MOZ_ALWAYS_INLINE void
PLDHashTable::Init(const PLDHashTableOps* aOps,
uint32_t aEntrySize, const fallible_t&, uint32_t aLength)
uint32_t aEntrySize, uint32_t aLength)
{
MOZ_ASSERT(!IsInitialized());

Expand All @@ -229,7 +226,7 @@ PLDHashTable::Init(const PLDHashTableOps* aOps,
MOZ_ASSERT(mEntryStore == nullptr);

if (aLength > PL_DHASH_MAX_INITIAL_LENGTH) {
return false;
MOZ_CRASH("Initial length is too large");
}

// Compute the smallest capacity allowing |aLength| elements to be inserted
Expand All @@ -243,51 +240,30 @@ PLDHashTable::Init(const PLDHashTableOps* aOps,

capacity = 1u << log2;
MOZ_ASSERT(capacity <= PL_DHASH_MAX_CAPACITY);
mOps = aOps;
mHashShift = PL_DHASH_BITS - log2;
mEntrySize = aEntrySize;
mEntryCount = mRemovedCount = 0;
mGeneration = 0;
uint32_t nbytes;
if (!SizeOfEntryStore(capacity, aEntrySize, &nbytes)) {
return false; // overflowed
MOZ_CRASH("Initial entry store size is too large");
}

mEntryStore = nullptr;

METER(memset(&mStats, 0, sizeof(mStats)));

// Set this only once we reach a point where we know we can't fail.
mOps = aOps;

#ifdef DEBUG
mRecursionLevel = 0;
#endif

return true;
}

bool
PL_DHashTableInit(PLDHashTable* aTable, const PLDHashTableOps* aOps,
uint32_t aEntrySize,
const fallible_t& aFallible, uint32_t aLength)
{
return aTable->Init(aOps, aEntrySize, aFallible, aLength);
}

void
PL_DHashTableInit(PLDHashTable* aTable, const PLDHashTableOps* aOps,
uint32_t aEntrySize, uint32_t aLength)
{
if (!PL_DHashTableInit(aTable, aOps, aEntrySize, fallible, aLength)) {
if (aLength > PL_DHASH_MAX_INITIAL_LENGTH) {
MOZ_CRASH(); // the asked-for length was too big
}
uint32_t capacity = MinCapacity(aLength), nbytes;
if (!SizeOfEntryStore(capacity, aEntrySize, &nbytes)) {
MOZ_CRASH(); // the required mEntryStore size was too big
}
NS_ABORT_OOM(nbytes); // allocation failed
}
aTable->Init(aOps, aEntrySize, aLength);
}

/*
Expand Down
14 changes: 2 additions & 12 deletions xpcom/glue/pldhash.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,7 @@ class PLDHashTable
uint32_t EntryCount() const { return mEntryCount; }
uint32_t Generation() const { return mGeneration; }

bool Init(const PLDHashTableOps* aOps, uint32_t aEntrySize,
const mozilla::fallible_t&, uint32_t aLength);
void Init(const PLDHashTableOps* aOps, uint32_t aEntrySize, uint32_t aLength);

void Finish();

Expand Down Expand Up @@ -467,7 +466,7 @@ const PLDHashTableOps* PL_DHashGetStubOps(void);

/*
* Dynamically allocate a new PLDHashTable, initialize it using
* PL_DHashTableInit, and return its address. Return null on allocation failure.
* PL_DHashTableInit, and return its address. Never returns null.
*/
PLDHashTable* PL_NewDHashTable(
const PLDHashTableOps* aOps, uint32_t aEntrySize,
Expand All @@ -494,15 +493,6 @@ void PL_DHashTableInit(
PLDHashTable* aTable, const PLDHashTableOps* aOps,
uint32_t aEntrySize, uint32_t aLength = PL_DHASH_DEFAULT_INITIAL_LENGTH);

/*
* Initialize aTable. This is the same as PL_DHashTableInit, except that it
* returns a boolean indicating success, rather than crashing on failure.
*/
MOZ_WARN_UNUSED_RESULT bool PL_DHashTableInit(
PLDHashTable* aTable, const PLDHashTableOps* aOps,
uint32_t aEntrySize, const mozilla::fallible_t&,
uint32_t aLength = PL_DHASH_DEFAULT_INITIAL_LENGTH);

/*
* Free |aTable|'s entry storage (via aTable->mOps->freeTable). Use this
* function to destroy a PLDHashTable that is allocated on the stack or in
Expand Down
83 changes: 14 additions & 69 deletions xpcom/tests/TestPLDHash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,20 @@ static bool test_pldhash_Init_capacity_ok()
}

// Try the largest allowed capacity. With PL_DHASH_MAX_CAPACITY==1<<26, this
// will allocate 0.5GB of entry store on 32-bit platforms and 1GB on 64-bit
// platforms.
if (!PL_DHashTableInit(&t, PL_DHashGetStubOps(), sizeof(PLDHashEntryStub),
mozilla::fallible, PL_DHASH_MAX_INITIAL_LENGTH)) {
return false;
}
// would allocate (if we added an element) 0.5GB of entry store on 32-bit
// platforms and 1GB on 64-bit platforms.
//
// Ideally we'd also try (a) a too-large capacity, and (b) a large capacity
// combined with a large entry size that when multipled overflow. But those
// cases would cause the test to abort immediately.
//
// Furthermore, ideally we'd also try a large-but-ok capacity that almost but
// doesn't quite overflow, but that would result in allocating just under 4GB
// of entry storage. That's very likely to fail on 32-bit platforms, so such
// a test wouldn't be reliable.
//
PL_DHashTableInit(&t, PL_DHashGetStubOps(), sizeof(PLDHashEntryStub),
PL_DHASH_MAX_INITIAL_LENGTH);

// Check that Init() sets |ops|.
if (!t.IsInitialized()) {
Expand All @@ -43,67 +51,6 @@ static bool test_pldhash_Init_capacity_ok()
return true;
}

static bool test_pldhash_Init_capacity_too_large()
{
PLDHashTable t;

// Check that the constructor nulls |ops|.
if (t.IsInitialized()) {
return false;
}

// Try the smallest too-large capacity.
if (PL_DHashTableInit(&t, PL_DHashGetStubOps(),
sizeof(PLDHashEntryStub),
mozilla::fallible,
PL_DHASH_MAX_INITIAL_LENGTH + 1)) {
return false; // it succeeded!?
}
// Don't call PL_DHashTableFinish() here; it's not safe after Init() failure.

// Check that |ops| is still null.
if (t.IsInitialized()) {
return false;
}

return true;
}

static bool test_pldhash_Init_overflow()
{
PLDHashTable t;

// Check that the constructor nulls |ops|.
if (t.IsInitialized()) {
return false;
}

// Try an acceptable capacity, but one whose byte size overflows uint32_t.
//
// Ideally we'd also try a large-but-ok capacity that almost but doesn't
// quite overflow, but that would result in allocating just under 4GB of
// entry storage. That's very likely to fail on 32-bit platforms, so such a
// test wouldn't be reliable.

struct OneKBEntry {
PLDHashEntryHdr hdr;
char buf[1024 - sizeof(PLDHashEntryHdr)];
};

if (PL_DHashTableInit(&t, PL_DHashGetStubOps(), sizeof(OneKBEntry),
mozilla::fallible, PL_DHASH_MAX_INITIAL_LENGTH)) {
return false; // it succeeded!?
}
// Don't call PL_DHashTableFinish() here; it's not safe after Init() failure.

// Check that |ops| is still null.
if (t.IsInitialized()) {
return false;
}

return true;
}

static bool test_pldhash_lazy_storage()
{
PLDHashTable t;
Expand Down Expand Up @@ -226,8 +173,6 @@ static const struct Test {
TestFunc func;
} tests[] = {
DECL_TEST(test_pldhash_Init_capacity_ok),
DECL_TEST(test_pldhash_Init_capacity_too_large),
DECL_TEST(test_pldhash_Init_overflow),
DECL_TEST(test_pldhash_lazy_storage),
// See bug 931062, we skip this test on Android due to OOM.
#ifndef MOZ_WIDGET_ANDROID
Expand Down

0 comments on commit e8e6640

Please sign in to comment.