Skip to content

Commit 2140adf

Browse files
laramielcopybara-github
authored andcommitted
In MemoryRegion, use swap on move-assignment.
PiperOrigin-RevId: 716714484 Change-Id: I309bbfb68f31941f60fad4c01cb62ffe992f8e29
1 parent df80595 commit 2140adf

File tree

4 files changed

+31
-6
lines changed

4 files changed

+31
-6
lines changed

tensorstore/internal/flat_cord_builder.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ namespace internal {
3838
/// likely to be retained.
3939
class FlatCordBuilder {
4040
public:
41-
FlatCordBuilder() = default;
4241
explicit FlatCordBuilder(size_t size) : FlatCordBuilder(size, size) {}
4342

4443
explicit FlatCordBuilder(internal_os::MemoryRegion region)

tensorstore/internal/os/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ tensorstore_cc_test(
297297
srcs = ["memory_region_test.cc"],
298298
deps = [
299299
":memory_region",
300+
"@com_google_absl//absl/strings:cord",
300301
"@com_google_googletest//:gtest_main",
301302
],
302303
)

tensorstore/internal/os/memory_region.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,21 @@ class MemoryRegion {
4040
MemoryRegion(const MemoryRegion&) = delete;
4141
MemoryRegion& operator=(const MemoryRegion&) = delete;
4242

43-
MemoryRegion(MemoryRegion&& other) { *this = std::move(other); }
43+
MemoryRegion(MemoryRegion&& other)
44+
: data_(std::exchange(other.data_, nullptr)),
45+
size_(std::exchange(other.size_, 0)),
46+
unmap_fn_(other.unmap_fn_) {}
47+
4448
MemoryRegion& operator=(MemoryRegion&& other) {
45-
data_ = std::exchange(other.data_, nullptr);
46-
size_ = std::exchange(other.size_, 0);
47-
unmap_fn_ = std::exchange(other.unmap_fn_, nullptr);
49+
std::swap(data_, other.data_);
50+
std::swap(size_, other.size_);
51+
std::swap(unmap_fn_, other.unmap_fn_);
4852
return *this;
4953
}
5054

5155
size_t size() const { return size_; }
52-
char* data() const { return data_; }
56+
const char* data() const { return data_; }
57+
char* data() { return data_; }
5358

5459
std::string_view as_string_view() const {
5560
return std::string_view(data_, size_);

tensorstore/internal/os/memory_region_test.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,35 @@
1616

1717
#include <stddef.h>
1818

19+
#include <utility>
20+
1921
#include <gtest/gtest.h>
22+
#include "absl/strings/cord.h"
2023

2124
using ::tensorstore::internal_os::AllocateHeapRegion;
2225

2326
namespace {
2427

28+
TEST(MemoryRegionTest, EmptyRegion) {
29+
auto region = AllocateHeapRegion(0);
30+
EXPECT_EQ(region.as_string_view().size(), 0);
31+
absl::Cord a = std::move(region).as_cord();
32+
}
33+
34+
TEST(MemoryRegionTest, Assignment) {
35+
auto region = AllocateHeapRegion(0);
36+
for (int i = 0; i < 10; ++i) region = AllocateHeapRegion(i);
37+
}
38+
2539
TEST(MemoryRegionTest, AllocateHeapRegion) {
2640
auto region = AllocateHeapRegion(16 * 1024 * 1024);
2741
EXPECT_EQ(region.as_string_view().size(), 16 * 1024 * 1024);
42+
43+
// Verify that assignment doesn't leak.
44+
region = AllocateHeapRegion(16);
45+
EXPECT_EQ(region.as_string_view().size(), 16);
46+
47+
absl::Cord a = std::move(region).as_cord();
2848
}
2949

3050
} // namespace

0 commit comments

Comments
 (0)