Skip to content

Commit

Permalink
AK+Everywhere: Move custom deleter capability to OwnPtr
Browse files Browse the repository at this point in the history
`OwnPtrWithCustomDeleter` was a decorator which provided the ability
to add a custom deleter to `OwnPtr` by wrapping and taking the deleter
as a run-time argument to the constructor. This solution means that no
additional space is needed for the `OwnPtr` because it doesn't need to
store a pointer to the deleter, but comes at the cost of having an
extra type that stores a pointer for every instance.

This logic is moved directly into `OwnPtr` by adding a template
argument that is defaulted to the default deleter for the type. This
means that the type itself stores the pointer to the deleter instead
of every instance and adds some type safety by encoding the deleter in
the type itself instead of taking a run-time argument.
  • Loading branch information
ldm5180 authored and trflynn89 committed Dec 17, 2022
1 parent 53133b4 commit f2336d0
Show file tree
Hide file tree
Showing 12 changed files with 74 additions and 63 deletions.
35 changes: 35 additions & 0 deletions AK/DefaultDelete.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2022, the SerenityOS developers.
*
* SPDX-License-Identifier: BSD-2-Clause
*/

#pragma once

namespace AK {

template<class T>
struct DefaultDelete {
constexpr DefaultDelete() = default;

constexpr void operator()(T* t)
{
delete t;
}
};

template<class T>
struct DefaultDelete<T[]> {
constexpr DefaultDelete() = default;

constexpr void operator()(T* t)
{
delete[] t;
}
};

}

#ifdef USING_AK_GLOBALLY
using AK::DefaultDelete;
#endif
3 changes: 2 additions & 1 deletion AK/Forward.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#pragma once

#include <AK/DefaultDelete.h>
#include <AK/Types.h>

namespace AK {
Expand Down Expand Up @@ -133,7 +134,7 @@ class LockRefPtr;
template<typename T>
class RefPtr;

template<typename T>
template<typename T, typename TDeleter = DefaultDelete<T>>
class OwnPtr;

template<typename T>
Expand Down
2 changes: 0 additions & 2 deletions AK/NonnullRefPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

namespace AK {

template<typename T>
class OwnPtr;
template<typename T>
class RefPtr;

Expand Down
5 changes: 3 additions & 2 deletions AK/OwnPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
#pragma once

#include <AK/Error.h>
#include <AK/Forward.h>
#include <AK/NonnullOwnPtr.h>
#include <AK/RefCounted.h>

#define OWNPTR_SCRUB_BYTE 0xf0

namespace AK {

template<typename T>
template<typename T, typename TDeleter>
class [[nodiscard]] OwnPtr {
public:
OwnPtr() = default;
Expand Down Expand Up @@ -105,7 +106,7 @@ class [[nodiscard]] OwnPtr {

void clear()
{
delete m_ptr;
TDeleter {}(m_ptr);
m_ptr = nullptr;
}

Expand Down
41 changes: 0 additions & 41 deletions AK/OwnPtrWithCustomDeleter.h

This file was deleted.

4 changes: 1 addition & 3 deletions AK/RefPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@
#include <AK/Atomic.h>
#include <AK/Error.h>
#include <AK/Format.h>
#include <AK/Forward.h>
#include <AK/NonnullRefPtr.h>
#include <AK/StdLibExtras.h>
#include <AK/Traits.h>
#include <AK/Types.h>

namespace AK {

template<typename T>
class OwnPtr;

template<typename T>
class [[nodiscard]] RefPtr {
template<typename U>
Expand Down
3 changes: 0 additions & 3 deletions Kernel/Library/LockRefPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@

namespace AK {

template<typename T>
class OwnPtr;

template<typename T>
struct LockRefPtrTraits {
ALWAYS_INLINE static T* as_ptr(FlatPtr bits)
Expand Down
2 changes: 0 additions & 2 deletions Kernel/Library/NonnullLockRefPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

namespace AK {

template<typename T>
class OwnPtr;
template<typename T, typename PtrTraits>
class LockRefPtr;

Expand Down
1 change: 1 addition & 0 deletions Tests/AK/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ set(AK_TEST_SOURCES
TestNonnullRefPtr.cpp
TestNumberFormat.cpp
TestOptional.cpp
TestOwnPtr.cpp
TestPrint.cpp
TestQueue.cpp
TestQuickSort.cpp
Expand Down
23 changes: 23 additions & 0 deletions Tests/AK/TestOwnPtr.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright (c) 2022, the SerenityOS developers.
*
* SPDX-License-Identifier: BSD-2-Clause
*/

#include <LibTest/TestCase.h>

#include <AK/OwnPtr.h>

static u64 deleter_call_count = 0;

TEST_CASE(should_call_custom_deleter)
{
auto deleter = [](auto* p) { if (p) ++deleter_call_count; };
auto ptr = OwnPtr<u64, decltype(deleter)> {};
ptr.clear();
EXPECT_EQ(0u, deleter_call_count);
ptr = adopt_own_if_nonnull(&deleter_call_count);
EXPECT_EQ(0u, deleter_call_count);
ptr.clear();
EXPECT_EQ(1u, deleter_call_count);
}
6 changes: 1 addition & 5 deletions Userland/Libraries/LibCore/System.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1334,11 +1334,7 @@ ErrorOr<AddressInfoVector> getaddrinfo(char const* nodename, char const* servnam
for (auto* result = results; result != nullptr; result = result->ai_next)
TRY(addresses.try_append(*result));

return AddressInfoVector { move(addresses), results,
[](struct addrinfo* ptr) {
if (ptr)
::freeaddrinfo(ptr);
} };
return AddressInfoVector { move(addresses), results };
}

ErrorOr<void> getsockopt(int sockfd, int level, int option, void* value, socklen_t* value_size)
Expand Down
12 changes: 8 additions & 4 deletions Userland/Libraries/LibCore/System.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#include <AK/Error.h>
#include <AK/Noncopyable.h>
#include <AK/OwnPtrWithCustomDeleter.h>
#include <AK/OwnPtr.h>
#include <AK/StringView.h>
#include <AK/Vector.h>
#include <dirent.h>
Expand Down Expand Up @@ -226,14 +226,18 @@ class AddressInfoVector {
private:
friend ErrorOr<AddressInfoVector> getaddrinfo(char const* nodename, char const* servname, struct addrinfo const& hints);

AddressInfoVector(Vector<struct addrinfo>&& addresses, struct addrinfo* ptr, AK::Function<void(struct addrinfo*)> deleter)
AddressInfoVector(Vector<struct addrinfo>&& addresses, struct addrinfo* ptr)
: m_addresses(move(addresses))
, m_ptr(ptr, move(deleter))
, m_ptr(adopt_own_if_nonnull(ptr))
{
}

struct AddrInfoDeleter {
void operator()(struct addrinfo* ptr) { ::freeaddrinfo(ptr); }
};

Vector<struct addrinfo> m_addresses {};
OwnPtrWithCustomDeleter<struct addrinfo> m_ptr;
OwnPtr<struct addrinfo, AddrInfoDeleter> m_ptr {};
};

ErrorOr<AddressInfoVector> getaddrinfo(char const* nodename, char const* servname, struct addrinfo const& hints);
Expand Down

0 comments on commit f2336d0

Please sign in to comment.