Skip to content

Commit 91e4489

Browse files
committed
refactor(buffer): rename 'additional' parameter to 'mem_to_free' for clarity
Header changes (include/libipc/buffer.h): - Rename: additional → mem_to_free (better semantic name) - Add documentation comments explaining the parameter's purpose - Clarifies that mem_to_free is passed to destructor instead of p - Use case: when data pointer is offset into a larger allocation Implementation changes (src/libipc/buffer.cpp): - Update parameter name in constructor implementation - No logic changes, just naming improvement Test changes (test/test_buffer.cpp): - Fix TEST_F(BufferTest, ConstructorWithMemToFree) - Previous test caused crash: passed stack variable address to destructor - New test correctly demonstrates the parameter's purpose: * Allocate 100-byte block * Use offset portion (bytes 25-75) as data * Destructor receives original block pointer for proper cleanup - Prevents double-free and invalid free errors Semantic explanation: buffer(data_ptr, size, destructor, mem_to_free) On destruction: destructor(mem_to_free ? mem_to_free : data_ptr, size) This allows: char* block = new char[100]; char* data = block + 25; buffer buf(data, 50, my_free, block); // Frees 'block', not 'data'
1 parent de76cf8 commit 91e4489

File tree

3 files changed

+21
-9
lines changed

3 files changed

+21
-9
lines changed

include/libipc/buffer.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ class IPC_EXPORT buffer {
1717
buffer();
1818

1919
buffer(void* p, std::size_t s, destructor_t d);
20-
buffer(void* p, std::size_t s, destructor_t d, void* additional);
20+
// mem_to_free: pointer to be passed to destructor (if different from p)
21+
// Use case: when p points into a larger allocated block that needs to be freed
22+
buffer(void* p, std::size_t s, destructor_t d, void* mem_to_free);
2123
buffer(void* p, std::size_t s);
2224

2325
template <std::size_t N>

src/libipc/buffer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ buffer::buffer(void* p, std::size_t s, destructor_t d)
3838
: p_(p_->make(p, s, d, nullptr)) {
3939
}
4040

41-
buffer::buffer(void* p, std::size_t s, destructor_t d, void* additional)
42-
: p_(p_->make(p, s, d, additional)) {
41+
buffer::buffer(void* p, std::size_t s, destructor_t d, void* mem_to_free)
42+
: p_(p_->make(p, s, d, mem_to_free)) {
4343
}
4444

4545
buffer::buffer(void* p, std::size_t s)

test/test_buffer.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,26 @@ TEST_F(BufferTest, DestructorCalled) {
7373
EXPECT_EQ(DestructorTracker::count, 1);
7474
}
7575

76-
// Test constructor with additional parameter
77-
TEST_F(BufferTest, ConstructorWithAdditional) {
78-
char* data = new char[50];
79-
int additional_value = 42;
76+
// Test constructor with mem_to_free parameter
77+
// Scenario: allocate a large block, but only use a portion as data
78+
TEST_F(BufferTest, ConstructorWithMemToFree) {
79+
// Allocate a block of 100 bytes
80+
char* allocated_block = new char[100];
8081

81-
buffer buf(data, 50, DestructorTracker::destructor, &additional_value);
82+
// But only use the middle 50 bytes as data (offset 25)
83+
char* data_start = allocated_block + 25;
84+
std::strcpy(data_start, "Offset data");
85+
86+
// When destroyed, should free the entire allocated_block, not just data_start
87+
buffer buf(data_start, 50, DestructorTracker::destructor, allocated_block);
8288

8389
EXPECT_FALSE(buf.empty());
8490
EXPECT_EQ(buf.size(), 50u);
85-
EXPECT_NE(buf.data(), nullptr);
91+
EXPECT_EQ(buf.data(), data_start);
92+
EXPECT_STREQ(static_cast<const char*>(buf.data()), "Offset data");
93+
94+
// Destructor will be called with allocated_block (not data_start)
95+
// This correctly frees the entire allocation
8696
}
8797

8898
// Test constructor without destructor

0 commit comments

Comments
 (0)