From c8deaa83fa6b8258635037aac1db272f05bbe471 Mon Sep 17 00:00:00 2001 From: Mateusz Moskala Date: Thu, 3 Apr 2025 18:22:38 +0200 Subject: [PATCH 1/5] Added project files --- homework/shared_ptr/CMakeLists.txt | 24 +++++ homework/shared_ptr/build.bat | 10 ++ homework/shared_ptr/format.bat | 1 + homework/shared_ptr/shared_ptr.hpp | 27 +++++ homework/shared_ptr/shared_ptr_tests.cpp | 131 +++++++++++++++++++++++ 5 files changed, 193 insertions(+) create mode 100644 homework/shared_ptr/CMakeLists.txt create mode 100644 homework/shared_ptr/build.bat create mode 100644 homework/shared_ptr/format.bat create mode 100644 homework/shared_ptr/shared_ptr.hpp create mode 100644 homework/shared_ptr/shared_ptr_tests.cpp diff --git a/homework/shared_ptr/CMakeLists.txt b/homework/shared_ptr/CMakeLists.txt new file mode 100644 index 00000000..10ef3fa8 --- /dev/null +++ b/homework/shared_ptr/CMakeLists.txt @@ -0,0 +1,24 @@ +cmake_minimum_required(VERSION 3.11.0) +project(shared_ptr) + +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + +include(FetchContent) +FetchContent_Declare( + googletest + GIT_REPOSITORY https://github.com/google/googletest.git + GIT_TAG main +) +# For Windows: Prevent overriding the parent project's compiler/linker settings +set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) +FetchContent_MakeAvailable(googletest) + +enable_testing() + +add_executable(${PROJECT_NAME}_tests ${PROJECT_NAME}_tests.cpp) + +target_link_libraries(${PROJECT_NAME}_tests gtest_main) + +include(GoogleTest) +gtest_discover_tests(${PROJECT_NAME}_tests) diff --git a/homework/shared_ptr/build.bat b/homework/shared_ptr/build.bat new file mode 100644 index 00000000..dc7cca9d --- /dev/null +++ b/homework/shared_ptr/build.bat @@ -0,0 +1,10 @@ +RMDIR build /S /Q +mkdir build +cd build +cmake -G "MinGW Makefiles" .. +MinGW32-make + +shared_ptr_tests.exe + +cd .. + diff --git a/homework/shared_ptr/format.bat b/homework/shared_ptr/format.bat new file mode 100644 index 00000000..5e62c1ff --- /dev/null +++ b/homework/shared_ptr/format.bat @@ -0,0 +1 @@ +clang-format --style=file -i *.hpp *.cpp \ No newline at end of file diff --git a/homework/shared_ptr/shared_ptr.hpp b/homework/shared_ptr/shared_ptr.hpp new file mode 100644 index 00000000..3e210c46 --- /dev/null +++ b/homework/shared_ptr/shared_ptr.hpp @@ -0,0 +1,27 @@ +/* +`shared_ptr` is a RAII class: + +* Holds a pointer to managed object (template class) +* Holds a pointer to shared control block with 2 counters and a deleter: + * shared_refs count (as `std::atomic`) + * weak_refs count (as `std::atomic`) + * deleter (function pointer) +* Constructor copies a pointer and allocate the control block +* Destructor decrease shared_refs and: + * if shared_refs == 0 -> release the managed object + * if shared_refs == 0 and weak_refs == 0 -> release the control block +* Copying is allowed - it increments shared_refs +* Moving is allowed and it means: + * Copying original pointers to a new object + * Setting source pointer to nullptr +* Member functions: `operator*()`, `operator->()`, `get()`, `reset()`, `use_count()`, `operator bool()` +* Should be implemented in `shared_ptr.hpp` file inside `my` namespace +* Tests should be written inside `shared_ptr_tests.cpp` using GoogleTest or Catch2 +* You should instantiate `shared_ptr` template class in `shared_ptr_tests.cpp` above your test cases, e.g. `template class my::shared_ptr;` It's needed for code coverage check to work properly. + +Do not forget about CI - UT + Valgrind / ASAN. Work in pairs. + +[Read the notes on cppreference.com](https://en.cppreference.com/w/cpp/memory/shared_ptr) +*/ + + diff --git a/homework/shared_ptr/shared_ptr_tests.cpp b/homework/shared_ptr/shared_ptr_tests.cpp new file mode 100644 index 00000000..f6e6f26e --- /dev/null +++ b/homework/shared_ptr/shared_ptr_tests.cpp @@ -0,0 +1,131 @@ +#include +#include "shared_ptr.hpp" + +// Instantiate the template class for testing +template class my::shared_ptr; + +TEST(SharedPtrTest, DefaultConstructor) { + my::shared_ptr sp; + EXPECT_EQ(sp.use_count(), 1); + EXPECT_FALSE(sp); +} + +TEST(SharedPtrTest, ConstructorWithPointer) { + my::shared_ptr sp(new int(42)); + EXPECT_EQ(sp.use_count(), 1); + EXPECT_TRUE(sp); + EXPECT_EQ(*sp, 42); +} + +TEST(SharedPtrTest, ArrowOperator) { + struct TestStruct { + int value; + TestStruct(int v) + : value(v) {} + }; + + my::shared_ptr sp(new TestStruct(42)); + EXPECT_EQ(sp->value, 42); + sp->value = 24; + EXPECT_EQ(sp->value, 24); +} + +TEST(SharedPtrTest, GetMethod) { + int* raw_ptr = new int(10); + my::shared_ptr sp(raw_ptr); + + EXPECT_EQ(sp.get(), raw_ptr); + EXPECT_EQ(*sp.get(), 10); + + EXPECT_EQ(sp.use_count(), 1); +} + +TEST(SharedPtrTest, ResetMethod) { + my::shared_ptr sp(new int(5)); + EXPECT_EQ(*sp, 5); + EXPECT_EQ(sp.use_count(), 1); + + sp.reset(new int(10)); + EXPECT_EQ(*sp, 10); + EXPECT_EQ(sp.use_count(), 1); + + sp.reset(); + EXPECT_EQ(sp.get(), nullptr); + EXPECT_EQ(sp.use_count(), 0); +} + +TEST(SharedPtrTest, ResetWithMultipleReferences) { + my::shared_ptr sp1(new int(5)); + my::shared_ptr sp2 = sp1; + + EXPECT_EQ(sp1.use_count(), 2); + EXPECT_EQ(sp2.use_count(), 2); + + sp1.reset(new int(10)); + EXPECT_EQ(*sp1, 10); + EXPECT_EQ(sp1.use_count(), 1); + EXPECT_EQ(*sp2, 5); + EXPECT_EQ(sp2.use_count(), 1); +} +TEST(SharedPtrTest, CopyConstructor) { + my::shared_ptr sp1(new int(42)); + my::shared_ptr sp2(sp1); + EXPECT_EQ(*sp1, 42); + EXPECT_EQ(*sp2, 42); + EXPECT_EQ(sp1.use_count(), 2); + EXPECT_EQ(sp2.use_count(), 2); +} + +TEST(SharedPtrTest, CopyAssignment) { + my::shared_ptr sp1(new int(42)); + my::shared_ptr sp2; + sp2 = sp1; + EXPECT_EQ(*sp1, 42); + EXPECT_EQ(*sp2, 42); + EXPECT_EQ(sp1.use_count(), 2); + EXPECT_EQ(sp2.use_count(), 2); +} + +TEST(SharedPtrTest, MoveConstructor) { + my::shared_ptr sp1(new int(42)); + my::shared_ptr sp2(std::move(sp1)); + EXPECT_EQ(*sp2, 42); + EXPECT_EQ(sp2.use_count(), 1); + EXPECT_FALSE(sp1); +} + +TEST(SharedPtrTest, MoveAssignment) { + my::shared_ptr sp1(new int(42)); + my::shared_ptr sp2; + sp2 = std::move(sp1); + EXPECT_EQ(*sp2, 42); + EXPECT_EQ(sp2.use_count(), 1); + EXPECT_FALSE(sp1); +} + +TEST(SharedPtrTest, BoolOperator) { + my::shared_ptr sp1(new int(42)); + my::shared_ptr sp2; + EXPECT_TRUE(sp1); + EXPECT_FALSE(sp2); +} + +TEST(SharedPtrTest, Swap) { + my::shared_ptr sp1(new int(42)); + my::shared_ptr sp2(new int(24)); + sp1.swap(sp2); + EXPECT_EQ(*sp1, 24); + EXPECT_EQ(*sp2, 42); +} + +TEST(SharedPtrTest, MultipleSharedPtrs) { + my::shared_ptr sp1(new int(42)); + { + my::shared_ptr sp2 = sp1; + my::shared_ptr sp3 = sp1; + EXPECT_EQ(sp1.use_count(), 3); + EXPECT_EQ(sp2.use_count(), 3); + EXPECT_EQ(sp3.use_count(), 3); + } + EXPECT_EQ(sp1.use_count(), 1); +} \ No newline at end of file From fcc728c9c0ae8e78a006dccfebcd60845f932dfd Mon Sep 17 00:00:00 2001 From: Mateusz Moskala Date: Fri, 4 Apr 2025 11:20:47 +0200 Subject: [PATCH 2/5] Added class implementation --- homework/shared_ptr/shared_ptr.hpp | 148 +++++++++++++++++++---- homework/shared_ptr/shared_ptr_tests.cpp | 9 +- 2 files changed, 124 insertions(+), 33 deletions(-) diff --git a/homework/shared_ptr/shared_ptr.hpp b/homework/shared_ptr/shared_ptr.hpp index 3e210c46..ee164b9a 100644 --- a/homework/shared_ptr/shared_ptr.hpp +++ b/homework/shared_ptr/shared_ptr.hpp @@ -1,27 +1,125 @@ -/* -`shared_ptr` is a RAII class: - -* Holds a pointer to managed object (template class) -* Holds a pointer to shared control block with 2 counters and a deleter: - * shared_refs count (as `std::atomic`) - * weak_refs count (as `std::atomic`) - * deleter (function pointer) -* Constructor copies a pointer and allocate the control block -* Destructor decrease shared_refs and: - * if shared_refs == 0 -> release the managed object - * if shared_refs == 0 and weak_refs == 0 -> release the control block -* Copying is allowed - it increments shared_refs -* Moving is allowed and it means: - * Copying original pointers to a new object - * Setting source pointer to nullptr -* Member functions: `operator*()`, `operator->()`, `get()`, `reset()`, `use_count()`, `operator bool()` -* Should be implemented in `shared_ptr.hpp` file inside `my` namespace -* Tests should be written inside `shared_ptr_tests.cpp` using GoogleTest or Catch2 -* You should instantiate `shared_ptr` template class in `shared_ptr_tests.cpp` above your test cases, e.g. `template class my::shared_ptr;` It's needed for code coverage check to work properly. - -Do not forget about CI - UT + Valgrind / ASAN. Work in pairs. - -[Read the notes on cppreference.com](https://en.cppreference.com/w/cpp/memory/shared_ptr) -*/ +#pragma once +namespace my { +template +class shared_ptr { +private: + // Holds a pointer to managed object (template class) + T* obj_ptr; + // Holds a pointer to shared control block with 2 counters and a deleter: + // * shared_refs count (as `std::atomic`) + // * weak_refs count (as `std::atomic`) + // * deleter (function pointer) + struct ControlBlock { + std::atomic shared_refs; + std::atomic weak_refs; + void (*deleter)(T*); + + ControlBlock() + : shared_refs(1), weak_refs(0), deleter(nullptr) {} + }; + ControlBlock* control_block_ptr; + +public: + // Constructor: copies a pointer and allocate the control block + shared_ptr(T* pointer = nullptr) { + obj_ptr = pointer; + control_block_ptr = new ControlBlock(); + } + // Destructor: decrease shared_refs and: + // - if shared_refs == 0 -> release the managed object + // - if shared_refs == 0 and weak_refs == 0 -> release the control block + ~shared_ptr() { + if (control_block_ptr) { + control_block_ptr->shared_refs--; + if (0 == control_block_ptr->shared_refs) { + if (control_block_ptr->deleter) { + control_block_ptr->deleter(obj_ptr); + } else { + delete obj_ptr; + } + if (0 == control_block_ptr->weak_refs) { + delete control_block_ptr; + } + } + } + } + // Copying is allowed - it increments shared_refs + shared_ptr(const shared_ptr& other) { + obj_ptr = other.obj_ptr; + + other.control_block_ptr->shared_refs++; + control_block_ptr = other.control_block_ptr; + } + shared_ptr& operator=(const shared_ptr& other) { + obj_ptr = other.obj_ptr; + + other.control_block_ptr->shared_refs++; + control_block_ptr = other.control_block_ptr; + + return *this; + } + // Moving is allowed and it means: + // * Copying original pointers to a new object + // * Setting source pointer to nullptr + shared_ptr(shared_ptr&& other) { + obj_ptr = other.obj_ptr; + control_block_ptr = other.control_block_ptr; + other.obj_ptr = nullptr; + other.control_block_ptr = nullptr; + } + shared_ptr& operator=(shared_ptr&& other) { + obj_ptr = other.obj_ptr; + control_block_ptr = other.control_block_ptr; + other.obj_ptr = nullptr; + other.control_block_ptr = nullptr; + return *this; + } + + size_t use_count() { + if (control_block_ptr) { + return control_block_ptr->shared_refs; + } else { + return 0; + } + } + + T& operator*() { + return *obj_ptr; + } + T* operator->() { + return obj_ptr; + } + explicit operator bool() const { + return obj_ptr != nullptr; + } + T* get() { + return obj_ptr; + } + void reset(T* new_ptr = nullptr) { + if (obj_ptr != new_ptr) { + if (control_block_ptr) { + control_block_ptr->shared_refs--; + if (0 == control_block_ptr->shared_refs) { + if (control_block_ptr->deleter) { + control_block_ptr->deleter(obj_ptr); + } else { + delete obj_ptr; + } + if (0 == control_block_ptr->weak_refs) { + delete control_block_ptr; + } + } + } + obj_ptr = new_ptr; + if (new_ptr) { + control_block_ptr = new ControlBlock(); + } else { + control_block_ptr = nullptr; + } + } + } +}; + +} // namespace my diff --git a/homework/shared_ptr/shared_ptr_tests.cpp b/homework/shared_ptr/shared_ptr_tests.cpp index f6e6f26e..1494ff4d 100644 --- a/homework/shared_ptr/shared_ptr_tests.cpp +++ b/homework/shared_ptr/shared_ptr_tests.cpp @@ -67,6 +67,7 @@ TEST(SharedPtrTest, ResetWithMultipleReferences) { EXPECT_EQ(*sp2, 5); EXPECT_EQ(sp2.use_count(), 1); } + TEST(SharedPtrTest, CopyConstructor) { my::shared_ptr sp1(new int(42)); my::shared_ptr sp2(sp1); @@ -110,14 +111,6 @@ TEST(SharedPtrTest, BoolOperator) { EXPECT_FALSE(sp2); } -TEST(SharedPtrTest, Swap) { - my::shared_ptr sp1(new int(42)); - my::shared_ptr sp2(new int(24)); - sp1.swap(sp2); - EXPECT_EQ(*sp1, 24); - EXPECT_EQ(*sp2, 42); -} - TEST(SharedPtrTest, MultipleSharedPtrs) { my::shared_ptr sp1(new int(42)); { From 66caf371340d7aa720ea8dfa73e756ab3b41bd5a Mon Sep 17 00:00:00 2001 From: Mateusz Moskala Date: Fri, 4 Apr 2025 11:26:02 +0200 Subject: [PATCH 3/5] Dummy commit to trigger pipeline --- homework/shared_ptr/shared_ptr.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/homework/shared_ptr/shared_ptr.hpp b/homework/shared_ptr/shared_ptr.hpp index ee164b9a..e90bc028 100644 --- a/homework/shared_ptr/shared_ptr.hpp +++ b/homework/shared_ptr/shared_ptr.hpp @@ -88,15 +88,19 @@ class shared_ptr { T& operator*() { return *obj_ptr; } + T* operator->() { return obj_ptr; } + explicit operator bool() const { return obj_ptr != nullptr; } + T* get() { return obj_ptr; } + void reset(T* new_ptr = nullptr) { if (obj_ptr != new_ptr) { if (control_block_ptr) { From e4c343820610607b46644f8607c30829155d3051 Mon Sep 17 00:00:00 2001 From: Mateusz Moskala Date: Fri, 4 Apr 2025 12:11:18 +0200 Subject: [PATCH 4/5] Fixed memory leaks in move constructor and move assignment --- homework/shared_ptr/shared_ptr.hpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/homework/shared_ptr/shared_ptr.hpp b/homework/shared_ptr/shared_ptr.hpp index e90bc028..5cdfe86b 100644 --- a/homework/shared_ptr/shared_ptr.hpp +++ b/homework/shared_ptr/shared_ptr.hpp @@ -63,17 +63,32 @@ class shared_ptr { // Moving is allowed and it means: // * Copying original pointers to a new object // * Setting source pointer to nullptr - shared_ptr(shared_ptr&& other) { - obj_ptr = other.obj_ptr; - control_block_ptr = other.control_block_ptr; + shared_ptr(shared_ptr&& other) + : obj_ptr(other.obj_ptr), control_block_ptr(other.control_block_ptr) { other.obj_ptr = nullptr; other.control_block_ptr = nullptr; } shared_ptr& operator=(shared_ptr&& other) { + if (control_block_ptr) { + control_block_ptr->shared_refs--; + if (0 == control_block_ptr->shared_refs) { + if (control_block_ptr->deleter) { + control_block_ptr->deleter(obj_ptr); + } else { + delete obj_ptr; + } + if (0 == control_block_ptr->weak_refs) { + delete control_block_ptr; + } + } + } + obj_ptr = other.obj_ptr; control_block_ptr = other.control_block_ptr; + other.obj_ptr = nullptr; other.control_block_ptr = nullptr; + return *this; } @@ -100,7 +115,7 @@ class shared_ptr { T* get() { return obj_ptr; } - + void reset(T* new_ptr = nullptr) { if (obj_ptr != new_ptr) { if (control_block_ptr) { From d762b0910a5399eef130258802e10ecef9e33cc0 Mon Sep 17 00:00:00 2001 From: Mateusz Moskala Date: Fri, 4 Apr 2025 12:25:45 +0200 Subject: [PATCH 5/5] Fixed memory leaks in copy assignment --- homework/shared_ptr/shared_ptr.hpp | 61 ++++++++++-------------------- 1 file changed, 20 insertions(+), 41 deletions(-) diff --git a/homework/shared_ptr/shared_ptr.hpp b/homework/shared_ptr/shared_ptr.hpp index 5cdfe86b..c7f16d2c 100644 --- a/homework/shared_ptr/shared_ptr.hpp +++ b/homework/shared_ptr/shared_ptr.hpp @@ -21,16 +21,7 @@ class shared_ptr { }; ControlBlock* control_block_ptr; -public: - // Constructor: copies a pointer and allocate the control block - shared_ptr(T* pointer = nullptr) { - obj_ptr = pointer; - control_block_ptr = new ControlBlock(); - } - // Destructor: decrease shared_refs and: - // - if shared_refs == 0 -> release the managed object - // - if shared_refs == 0 and weak_refs == 0 -> release the control block - ~shared_ptr() { + void remove_this_pointer() { if (control_block_ptr) { control_block_ptr->shared_refs--; if (0 == control_block_ptr->shared_refs) { @@ -45,19 +36,31 @@ class shared_ptr { } } } + +public: + // Constructor: copies a pointer and allocate the control block + shared_ptr(T* pointer = nullptr) { + obj_ptr = pointer; + control_block_ptr = new ControlBlock(); + } + // Destructor: decrease shared_refs and: + // - if shared_refs == 0 -> release the managed object + // - if shared_refs == 0 and weak_refs == 0 -> release the control block + ~shared_ptr() { + remove_this_pointer(); + } // Copying is allowed - it increments shared_refs shared_ptr(const shared_ptr& other) { obj_ptr = other.obj_ptr; - - other.control_block_ptr->shared_refs++; control_block_ptr = other.control_block_ptr; + other.control_block_ptr->shared_refs++; } shared_ptr& operator=(const shared_ptr& other) { + remove_this_pointer(); obj_ptr = other.obj_ptr; - - other.control_block_ptr->shared_refs++; control_block_ptr = other.control_block_ptr; - + if (control_block_ptr) + control_block_ptr->shared_refs++; return *this; } // Moving is allowed and it means: @@ -69,19 +72,7 @@ class shared_ptr { other.control_block_ptr = nullptr; } shared_ptr& operator=(shared_ptr&& other) { - if (control_block_ptr) { - control_block_ptr->shared_refs--; - if (0 == control_block_ptr->shared_refs) { - if (control_block_ptr->deleter) { - control_block_ptr->deleter(obj_ptr); - } else { - delete obj_ptr; - } - if (0 == control_block_ptr->weak_refs) { - delete control_block_ptr; - } - } - } + remove_this_pointer(); obj_ptr = other.obj_ptr; control_block_ptr = other.control_block_ptr; @@ -118,19 +109,7 @@ class shared_ptr { void reset(T* new_ptr = nullptr) { if (obj_ptr != new_ptr) { - if (control_block_ptr) { - control_block_ptr->shared_refs--; - if (0 == control_block_ptr->shared_refs) { - if (control_block_ptr->deleter) { - control_block_ptr->deleter(obj_ptr); - } else { - delete obj_ptr; - } - if (0 == control_block_ptr->weak_refs) { - delete control_block_ptr; - } - } - } + remove_this_pointer(); obj_ptr = new_ptr; if (new_ptr) { control_block_ptr = new ControlBlock();