make BpfMap.reset() harder to use
Long term we don't even want reset() to exist. It's simply not useful since only the bpfloader can create maps, and thus all we can do is construct from pre-existing pinned bpf path - and we should never destroy/unpin/delete these. Similarly the move constructor is probably spurious. Basically the only real use for reset() is in tests, and even there the overall utility is not clear. Hence, in the mean time, make it harder to use incorrectly, and harder to use in general. To do that we get rid of reset() -> you must call reset(-1) and we eliminate the ability to call reset(unique_fd) which has unclear fd-leaking semantics. Also remove a spot where it is called spuriously. Test: build, atest Bug: 129773125 Signed-off-by: Maciej Żenczykowski <maze@google.com> Change-Id: Ia018224aca0005ab68809b350595485ccd3f1bd3
This commit is contained in:
parent
cd5c3023ce
commit
4f6573959b
2 changed files with 5 additions and 6 deletions
|
|
@ -140,7 +140,7 @@ TEST_F(BpfMapTest, reset) {
|
|||
uint32_t value_write = TEST_VALUE1;
|
||||
writeToMapAndCheck(testMap, key, value_write);
|
||||
|
||||
testMap.reset();
|
||||
testMap.reset(-1);
|
||||
checkMapInvalid(testMap);
|
||||
ASSERT_GT(0, findMapEntry(testMap.getMap(), &key, &value_write));
|
||||
ASSERT_EQ(EBADF, errno);
|
||||
|
|
|
|||
|
|
@ -131,12 +131,12 @@ class BpfMap {
|
|||
// Move constructor
|
||||
void operator=(BpfMap<Key, Value>&& other) noexcept {
|
||||
mMapFd = std::move(other.mMapFd);
|
||||
other.reset();
|
||||
other.reset(-1);
|
||||
}
|
||||
|
||||
void reset(int fd = -1) {
|
||||
mMapFd.reset(fd);
|
||||
}
|
||||
void reset(base::unique_fd fd) = delete;
|
||||
|
||||
void reset(int fd) { mMapFd.reset(fd); }
|
||||
|
||||
bool isValid() const { return mMapFd != -1; }
|
||||
|
||||
|
|
@ -172,7 +172,6 @@ template <class Key, class Value>
|
|||
base::Result<void> BpfMap<Key, Value>::init(const char* path) {
|
||||
mMapFd = base::unique_fd(mapRetrieve(path, 0));
|
||||
if (mMapFd == -1) {
|
||||
reset();
|
||||
return base::ErrnoErrorf("Pinned map not accessible or does not exist: ({})", path);
|
||||
}
|
||||
return {};
|
||||
|
|
|
|||
Loading…
Reference in a new issue