facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-07 07:17:55 +03:00
Code Issues Projects Releases Wiki Activity

add explicit conversion from std::unique_ptr to InodePtr

Browse Source 
Summary:
To make it clearer why newPtrLocked is safe during the Inode class
construction process, add a takeOwnership function (better name
suggestions welcome) that converts from the newly-instantiated
unique_ptr to a managed InodePtr.

Reviewed By: simpkins

Differential Revision: D7204818

fbshipit-source-id: 8a40189588442490120623da86195c6fc99c9c51
This commit is contained in:
Chad Austin 2018-03-09 13:32:42 -08:00 committed by Facebook Github Bot
parent 9bd173054b
commit 8fbe2f85fa
2 changed files with 19 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
eden/fs/inodes/InodePtr.h
View File

@ -13,6 +13,7 @@
#include <glog/logging.h>
#include <cstddef>
#include <memory>
#include <utility>
namespace facebook {
@ -126,6 +127,13 @@ class InodePtrImpl {
return InodePtrImpl{value, LOCKED_INCREMENT};
}
/**
* Like newPtrLocked() but consumes the given unique_ptr.
*/
static InodePtrImpl takeOwnership(std::unique_ptr<InodeType> value) noexcept {
return InodePtrImpl{value.release(), LOCKED_INCREMENT};
}
/**
* An API for TreeInode to use to construct an InodePtr from itself in order
* to give to new children inodes that it creates.
@ -234,12 +242,15 @@ class InodePtr : public InodePtrImpl<InodeBase> {
}
/*
* Override newPtrLocked() and newPtrFromExisting() to return an
* InodePtr instead of the InodePtrImpl parent class.
* Override newPtrLocked(), takeOwnership(), and newPtrFromExisting() to
* return an InodePtr instead of the InodePtrImpl parent class.
*/
static InodePtr newPtrLocked(InodeBase* value) noexcept {
return InodePtr{value, InodePtrImpl<InodeBase>::LOCKED_INCREMENT};
}
static InodePtr takeOwnership(std::unique_ptr<InodeBase> value) noexcept {
return InodePtr::newPtrLocked(value.release());
}
static InodePtr newPtrFromExisting(InodeBase* value) noexcept {
return InodePtr{value, InodePtrImpl<InodeBase>::NORMAL_INCREMENT};
}

12
eden/fs/inodes/TreeInode.cpp
View File

@ -243,7 +243,7 @@ Future<InodePtr> TreeInode::getOrLoadChild(PathComponentPiece name) {
auto childInode = loadFuture.get();
entryPtr.setInode(childInode.get());
promises = getInodeMap()->inodeLoadComplete(childInode.get());
childInodePtr = InodePtr::newPtrLocked(childInode.release());
childInodePtr = InodePtr::takeOwnership(std::move(childInode));
} else {
inodeLoadFuture = std::move(loadFuture);
}
@ -310,11 +310,11 @@ class LookupProcessor {
Future<InodePtr> TreeInode::getChildRecursive(RelativePathPiece path) {
auto pathStr = path.stringPiece();
if (pathStr.empty()) {
return makeFuture<InodePtr>(InodePtr::newPtrFromExisting(this));
return makeFuture<InodePtr>(inodePtrFromThis());
}
auto processor = std::make_unique<LookupProcessor>(path);
auto future = processor->next(TreeInodePtr::newPtrFromExisting(this));
auto future = processor->next(inodePtrFromThis());
// This ensure() callback serves to hold onto the unique_ptr,
// and makes sure it only gets destroyed when the future is finally resolved.
return future.ensure([p = std::move(processor)]() mutable { p.reset(); });
@ -354,7 +354,7 @@ void TreeInode::loadUnlinkedChildInode(
auto file = std::make_unique<FileInode>(
number, inodePtrFromThis(), name, mode, hash);
promises = getInodeMap()->inodeLoadComplete(file.get());
inodePtr = InodePtr::newPtrLocked(file.release());
inodePtr = InodePtr::takeOwnership(std::move(file));
} else {
Dir dir;
@ -380,7 +380,7 @@ void TreeInode::loadUnlinkedChildInode(
auto tree = std::make_unique<TreeInode>(
number, inodePtrFromThis(), name, std::move(dir));
promises = getInodeMap()->inodeLoadComplete(tree.get());
inodePtr = InodePtr::newPtrLocked(tree.release());
inodePtr = InodePtr::takeOwnership(std::move(tree));
}
inodePtr->markUnlinkedAfterLoad();
@ -489,7 +489,7 @@ void TreeInode::inodeLoadComplete(
}
// Fulfill all of the pending promises after releasing our lock
auto inodePtr = InodePtr::newPtrLocked(childInode.release());
auto inodePtr = InodePtr::takeOwnership(std::move(childInode));
for (auto& promise : promises) {
promise.setValue(inodePtr);
}