ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-04 19:25:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Eliminate code duplication and improve documentation:

Browse Source 
The legacy functions `cdirFirst` and `dirFirst` were mostly
identical; the differences were only type-related. The same
situation existed with `cdirNext` and `dirNext`.

This commit removes the duplicated code by introducing new
template functions that abstract away the differences that
are present between each pair of functions.

This commit also improves the naming of function arguments,
helping to elucidate their purpose & use and to make the
code self-documenting.
This commit is contained in:
Nik Bougalis
2021-09-03 03:58:45 -07:00
parent c231adf324
commit 234b754038
7 changed files with 187 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/ripple/app/misc/NetworkOPs.cpp
View File

@@ -3410,13 +3410,7 @@ NetworkOPsImp::getBookPage(
uTipIndex = sleOfferDir->key();
saDirRate = amountFromQuality(getQuality(uTipIndex));
cdirFirst(
view,
uTipIndex,
sleOfferDir,
uBookEntry,
offerIndex,
viewJ);
cdirFirst(view, uTipIndex, sleOfferDir, uBookEntry, offerIndex);
JLOG(m_journal.trace())
<< "getBookPage: uTipIndex=" << uTipIndex;
@@ -3536,13 +3530,7 @@ NetworkOPsImp::getBookPage(
JLOG(m_journal.warn()) << "Missing offer";
}
if (!cdirNext(
view,
uTipIndex,
sleOfferDir,
uBookEntry,
offerIndex,
viewJ))
if (!cdirNext(view, uTipIndex, sleOfferDir, uBookEntry, offerIndex))
{
bDirectAdvance = true;
}

2
src/ripple/app/tx/impl/BookTip.cpp
View File

@@ -55,7 +55,7 @@ BookTip::step(beast::Journal j)
unsigned int di = 0;
std::shared_ptr<SLE> dir;
if (dirFirst(view_, *first_page, dir, di, m_index, j))
if (dirFirst(view_, *first_page, dir, di, m_index))
{
m_dir = dir->key();
m_entry = view_.peek(keylet::offer(m_index));

14
src/ripple/app/tx/impl/DeleteAccount.cpp
View File

@@ -198,12 +198,7 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
uint256 dirEntry{beast::zero};
if (!cdirFirst(
ctx.view,
ownerDirKeylet.key,
sleDirNode,
uDirEntry,
dirEntry,
ctx.j))
ctx.view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry))
// Account has no directory at all. This _should_ have been caught
// by the dirIsEmpty() check earlier, but it's okay to catch it here.
return tesSUCCESS;
@@ -237,7 +232,7 @@ DeleteAccount::preclaim(PreclaimContext const& ctx)
return tefTOO_BIG;
} while (cdirNext(
ctx.view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry, ctx.j));
ctx.view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry));
return tesSUCCESS;
}
@@ -261,8 +256,7 @@ DeleteAccount::doApply()
uint256 dirEntry{beast::zero};
if (view().exists(ownerDirKeylet) &&
dirFirst(
view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry, j_))
dirFirst(view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry))
{
do
{
@@ -324,7 +318,7 @@ DeleteAccount::doApply()
uDirEntry = 0;
} while (dirNext(
view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry, j_));
view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry));
}
// Transfer any XRP remaining after the fee is paid to the destination:

96
src/ripple/ledger/View.h
View File

@@ -125,30 +125,6 @@ transferRate(ReadView const& view, AccountID const& issuer);
[[nodiscard]] bool
dirIsEmpty(ReadView const& view, Keylet const& k);
// Return the first entry and advance uDirEntry.
// <-- true, if had a next entry.
// VFALCO Fix these clumsy routines with an iterator
bool
cdirFirst(
ReadView const& view,
uint256 const& uRootIndex, // --> Root of directory.
std::shared_ptr<SLE const>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-- next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j);
// Return the current entry and advance uDirEntry.
// <-- true, if had a next entry.
// VFALCO Fix these clumsy routines with an iterator
bool
cdirNext(
ReadView const& view,
uint256 const& uRootIndex, // --> Root of directory
std::shared_ptr<SLE const>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-> next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j);
// Return the list of enabled amendments
[[nodiscard]] std::set<uint256>
getEnabledAmendments(ReadView const& view);
@@ -222,29 +198,69 @@ adjustOwnerCount(
std::int32_t amount,
beast::Journal j);
// Return the first entry and advance uDirEntry.
// <-- true, if had a next entry.
// VFALCO Fix these clumsy routines with an iterator
/** @{ */
/** Returns the first entry in the directory, advancing the index
@deprecated These are legacy function that are considered deprecated
and will soon be replaced with an iterator-based model
that is easier to use. You should not use them in new code.
@param view The view against which to operate
@param root The root (i.e. first page) of the directory to iterate
@param page The current page
@param index The index inside the current page
@param entry The entry at the current index
@return true if the directory isn't empty; false otherwise
*/
bool
cdirFirst(
ReadView const& view,
uint256 const& root,
std::shared_ptr<SLE const>& page,
unsigned int& index,
uint256& entry);
bool
dirFirst(
ApplyView& view,
uint256 const& uRootIndex, // --> Root of directory.
std::shared_ptr<SLE>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-- next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j);
uint256 const& root,
std::shared_ptr<SLE>& page,
unsigned int& index,
uint256& entry);
/** @} */
/** @{ */
/** Returns the next entry in the directory, advancing the index
@deprecated These are legacy function that are considered deprecated
and will soon be replaced with an iterator-based model
that is easier to use. You should not use them in new code.
@param view The view against which to operate
@param root The root (i.e. first page) of the directory to iterate
@param page The current page
@param index The index inside the current page
@param entry The entry at the current index
@return true if the directory isn't empty; false otherwise
*/
bool
cdirNext(
ReadView const& view,
uint256 const& root,
std::shared_ptr<SLE const>& page,
unsigned int& index,
uint256& entry);
// Return the current entry and advance uDirEntry.
// <-- true, if had a next entry.
// VFALCO Fix these clumsy routines with an iterator
bool
dirNext(
ApplyView& view,
uint256 const& uRootIndex, // --> Root of directory
std::shared_ptr<SLE>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-> next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j);
uint256 const& root,
std::shared_ptr<SLE>& page,
unsigned int& index,
uint256& entry);
/** @} */
[[nodiscard]] std::function<void(SLE::ref)>
describeOwnerDir(AccountID const& account);

12
src/ripple/ledger/impl/BookDirs.cpp
View File

@@ -33,13 +33,7 @@ BookDirs::BookDirs(ReadView const& view, Book const& book)
assert(root_ != beast::zero);
if (key_ != beast::zero)
{
if (!cdirFirst(
*view_,
key_,
sle_,
entry_,
index_,
beast::Journal{beast::Journal::getNullSink()}))
if (!cdirFirst(*view_, key_, sle_, entry_, index_))
{
assert(false);
}
@@ -96,7 +90,7 @@ BookDirs::const_iterator::operator++()
using beast::zero;
assert(index_ != zero);
if (!cdirNext(*view_, cur_key_, sle_, entry_, index_, j_))
if (!cdirNext(*view_, cur_key_, sle_, entry_, index_))
{
if (index_ != 0 ||
(cur_key_ =
@@ -106,7 +100,7 @@ BookDirs::const_iterator::operator++()
entry_ = 0;
index_ = zero;
}
else if (!cdirFirst(*view_, cur_key_, sle_, entry_, index_, j_))
else if (!cdirFirst(*view_, cur_key_, sle_, entry_, index_))
{
assert(false);
}

226
src/ripple/ledger/impl/View.cpp
View File

@@ -34,6 +34,126 @@
namespace ripple {
namespace detail {
template <
class V,
class N,
class = std::enable_if_t<
std::is_same_v<std::remove_cv_t<N>, SLE> &&
std::is_base_of_v<ReadView, V>>>
bool
internalDirNext(
V& view,
uint256 const& root,
std::shared_ptr<N>& page,
unsigned int& index,
uint256& entry)
{
auto const& svIndexes = page->getFieldV256(sfIndexes);
assert(index <= svIndexes.size());
if (index >= svIndexes.size())
{
auto const next = page->getFieldU64(sfIndexNext);
if (!next)
{
entry.zero();
return false;
}
if constexpr (std::is_const_v<N>)
page = view.read(keylet::page(root, next));
else
page = view.peek(keylet::page(root, next));
assert(page);
if (!page)
return false;
index = 0;
return internalDirNext(view, root, page, index, entry);
}
entry = svIndexes[index++];
return true;
}
template <
class V,
class N,
class = std::enable_if_t<
std::is_same_v<std::remove_cv_t<N>, SLE> &&
std::is_base_of_v<ReadView, V>>>
bool
internalDirFirst(
V& view,
uint256 const& root,
std::shared_ptr<N>& page,
unsigned int& index,
uint256& entry)
{
if constexpr (std::is_const_v<N>)
page = view.read(keylet::page(root));
else
page = view.peek(keylet::page(root));
assert(page);
index = 0;
return internalDirNext(view, root, page, index, entry);
}
} // namespace detail
bool
dirFirst(
ApplyView& view,
uint256 const& root,
std::shared_ptr<SLE>& page,
unsigned int& index,
uint256& entry)
{
return detail::internalDirFirst(view, root, page, index, entry);
}
bool
dirNext(
ApplyView& view,
uint256 const& root,
std::shared_ptr<SLE>& page,
unsigned int& index,
uint256& entry)
{
return detail::internalDirNext(view, root, page, index, entry);
}
bool
cdirFirst(
ReadView const& view,
uint256 const& root,
std::shared_ptr<SLE const>& page,
unsigned int& index,
uint256& entry)
{
return detail::internalDirFirst(view, root, page, index, entry);
}
bool
cdirNext(
ReadView const& view,
uint256 const& root,
std::shared_ptr<SLE const>& page,
unsigned int& index,
uint256& entry)
{
return detail::internalDirNext(view, root, page, index, entry);
}
//------------------------------------------------------------------------------
//
// Observers
@@ -480,59 +600,6 @@ dirIsEmpty(ReadView const& view, Keylet const& k)
return sleNode->getFieldU64(sfIndexNext) == 0;
}
bool
cdirFirst(
ReadView const& view,
uint256 const& uRootIndex, // --> Root of directory.
std::shared_ptr<SLE const>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-- next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j)
{
sleNode = view.read(keylet::page(uRootIndex));
uDirEntry = 0;
assert(sleNode); // Never probe for directories.
return cdirNext(view, uRootIndex, sleNode, uDirEntry, uEntryIndex, j);
}
bool
cdirNext(
ReadView const& view,
uint256 const& uRootIndex, // --> Root of directory
std::shared_ptr<SLE const>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-> next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j)
{
auto const& svIndexes = sleNode->getFieldV256(sfIndexes);
assert(uDirEntry <= svIndexes.size());
if (uDirEntry >= svIndexes.size())
{
auto const uNodeNext = sleNode->getFieldU64(sfIndexNext);
if (!uNodeNext)
{
uEntryIndex.zero();
return false;
}
auto const sleNext = view.read(keylet::page(uRootIndex, uNodeNext));
uDirEntry = 0;
if (!sleNext)
{
// This should never happen
JLOG(j.fatal()) << "Corrupt directory: index:" << uRootIndex
<< " next:" << uNodeNext;
return false;
}
sleNode = sleNext;
return cdirNext(view, uRootIndex, sleNode, uDirEntry, uEntryIndex, j);
}
uEntryIndex = svIndexes[uDirEntry++];
JLOG(j.trace()) << "dirNext:"
<< " uDirEntry=" << uDirEntry
<< " uEntryIndex=" << uEntryIndex;
return true;
}
std::set<uint256>
getEnabledAmendments(ReadView const& view)
{
@@ -660,59 +727,6 @@ adjustOwnerCount(
view.update(sle);
}
bool
dirFirst(
ApplyView& view,
uint256 const& uRootIndex, // --> Root of directory.
std::shared_ptr<SLE>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-- next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j)
{
sleNode = view.peek(keylet::page(uRootIndex));
uDirEntry = 0;
assert(sleNode); // Never probe for directories.
return dirNext(view, uRootIndex, sleNode, uDirEntry, uEntryIndex, j);
}
bool
dirNext(
ApplyView& view,
uint256 const& uRootIndex, // --> Root of directory
std::shared_ptr<SLE>& sleNode, // <-> current node
unsigned int& uDirEntry, // <-> next entry
uint256& uEntryIndex, // <-- The entry, if available. Otherwise, zero.
beast::Journal j)
{
auto const& svIndexes = sleNode->getFieldV256(sfIndexes);
assert(uDirEntry <= svIndexes.size());
if (uDirEntry >= svIndexes.size())
{
auto const uNodeNext = sleNode->getFieldU64(sfIndexNext);
if (!uNodeNext)
{
uEntryIndex.zero();
return false;
}
auto const sleNext = view.peek(keylet::page(uRootIndex, uNodeNext));
uDirEntry = 0;
if (!sleNext)
{
// This should never happen
JLOG(j.fatal()) << "Corrupt directory: index:" << uRootIndex
<< " next:" << uNodeNext;
return false;
}
sleNode = sleNext;
return dirNext(view, uRootIndex, sleNode, uDirEntry, uEntryIndex, j);
}
uEntryIndex = svIndexes[uDirEntry++];
JLOG(j.trace()) << "dirNext:"
<< " uDirEntry=" << uDirEntry
<< " uEntryIndex=" << uEntryIndex;
return true;
}
std::function<void(SLE::ref)>
describeOwnerDir(AccountID const& account)
{

7
src/test/rpc/Book_test.cpp
View File

@@ -41,12 +41,7 @@ class Book_test : public beast::unit_test::suite
uint256 offerIndex;
unsigned int bookEntry;
cdirFirst(
*view,
sleOfferDir->key(),
sleOfferDir,
bookEntry,
offerIndex,
env.journal);
*view, sleOfferDir->key(), sleOfferDir, bookEntry, offerIndex);
auto sleOffer = view->read(keylet::offer(offerIndex));
dir = to_string(sleOffer->getFieldH256(sfBookDirectory));
}