Closed
Bug 1121760
Opened 9 years ago
Closed 9 years ago
Remove PL_DHashTable* functions
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(6 files, 1 obsolete file)
67.50 KB,
patch
|
poiru
:
review+
|
Details | Diff | Splinter Review |
58.43 KB,
patch
|
poiru
:
review+
|
Details | Diff | Splinter Review |
27.36 KB,
patch
|
poiru
:
review+
|
Details | Diff | Splinter Review |
6.55 KB,
patch
|
poiru
:
review+
|
Details | Diff | Splinter Review |
3.46 KB,
patch
|
poiru
:
review+
|
Details | Diff | Splinter Review |
49.07 KB,
patch
|
poiru
:
review+
|
Details | Diff | Splinter Review |
PLDHashTable now has both C-style functions (e.g. PL_DHashTableInit()) and C++-style methods (e.g. PLDHashTable::Init()). It would be nice to remove the former to avoid code duplication and make the style more like the rest of the codebase.
Assignee | ||
Updated•9 years ago
|
Summary: Remove PL_DHashTable* methods → Remove PL_DHashTable* functions
Assignee | ||
Comment 1•9 years ago
|
||
I tried removing PL_DHashTableInit() just to see if there were any unexpected difficulties. There weren't. Michael, is this something you'd be interested in continuing? If so, it'd probably make sense to put each function's removal in a separate patch (to simplify reviewing) but then land them all at once (to simplify updating comm-central).
Assignee | ||
Comment 2•9 years ago
|
||
I'll take this, but I have a whole lot of other PLDHashTable clean-up patches I want to land first.
Assignee: nobody → n.nethercote
Blocks: modernize-pldhash
Assignee | ||
Updated•9 years ago
|
Attachment #8549265 -
Attachment is obsolete: true
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8660767 -
Flags: review?(birunthan)
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8660780 -
Flags: review?(birunthan)
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8660781 -
Flags: review?(birunthan)
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8660786 -
Flags: review?(birunthan)
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8660787 -
Flags: review?(birunthan)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8660788 -
Flags: review?(birunthan)
Updated•9 years ago
|
Attachment #8660767 -
Flags: review?(birunthan) → review+
Comment 9•9 years ago
|
||
Comment on attachment 8660780 [details] [diff] [review] (part 2) - Remove PL_DHashTableAdd() Review of attachment 8660780 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/cache/nsDiskCacheBinding.cpp @@ +229,5 @@ > NS_ASSERTION(initialized, "nsDiskCacheBindery not initialized"); > > // find hash entry for key > + auto hashEntry = static_cast<HashTableEntry*> > + (table.Add((void *)(uintptr_t) binding->mRecord.HashNumber(), fallible)); Could you remove the space before the * and after the ) while you're here?
Attachment #8660780 -
Flags: review?(birunthan) → review+
Updated•9 years ago
|
Attachment #8660781 -
Flags: review?(birunthan) → review+
Updated•9 years ago
|
Attachment #8660786 -
Flags: review?(birunthan) → review+
Updated•9 years ago
|
Attachment #8660787 -
Flags: review?(birunthan) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8660788 [details] [diff] [review] (part 6) - Move all remaining PL_DHash*() functions into PLDHashTable Review of attachment 8660788 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/glue/pldhash.cpp @@ +119,5 @@ > nullptr > }; > > +/* static */ const PLDHashTableOps* > +PLDHashTable::StubOps(void) Please remove `void`.
Attachment #8660788 -
Flags: review?(birunthan) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Thank you for the lighting reviews :)
Assignee | ||
Comment 12•9 years ago
|
||
Er, that should be *lightning*...
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e37d0c2d053a https://hg.mozilla.org/integration/mozilla-inbound/rev/16597175b6f3 https://hg.mozilla.org/integration/mozilla-inbound/rev/108f4bf0585b https://hg.mozilla.org/integration/mozilla-inbound/rev/f957524fc624 https://hg.mozilla.org/integration/mozilla-inbound/rev/507728088761 https://hg.mozilla.org/integration/mozilla-inbound/rev/36f11da8d2d9
https://hg.mozilla.org/mozilla-central/rev/e37d0c2d053a https://hg.mozilla.org/mozilla-central/rev/16597175b6f3 https://hg.mozilla.org/mozilla-central/rev/108f4bf0585b https://hg.mozilla.org/mozilla-central/rev/f957524fc624 https://hg.mozilla.org/mozilla-central/rev/507728088761 https://hg.mozilla.org/mozilla-central/rev/36f11da8d2d9
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•