Skip to content

Commit 4cd35bd

Browse files
umeganeCopilot
andcommitted
refactor(replication): avoid blob restore warnings and clean up byte I/O
Move internal blob path resolution behind datastore_impl so TCP and RDMA replication restore paths no longer depend on the public ready-checked datastore::get_blob_file() API. Also clean up byte-buffer handling in blob send paths to address clang-tidy warnings without adding extra copies in the hot path. Co-authored-by: Copilot <copilot@github.com>
1 parent 9647317 commit 4cd35bd

5 files changed

Lines changed: 16 additions & 11 deletions

File tree

src/limestone/rdma/rdma_socket_io.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
#include <algorithm>
2020
#include <array>
21-
#include <cstring>
21+
#include <iterator>
2222
#include <vector>
2323

2424
#include "replication/blob_send_utils.h"
@@ -94,15 +94,15 @@ rdma_send_stream_base::buffer_fill_result rdma_socket_io::fill_blob_header_and_f
9494
if (encoded.size() != blob_header_size) {
9595
return {false, "encoded blob header size mismatch"};
9696
}
97-
std::memcpy(buffer, encoded.data(), encoded.size());
97+
std::copy(encoded.begin(), encoded.end(), buffer);
9898
auto const payload_capacity = capacity - blob_header_size;
9999
if (payload_capacity > 0U) {
100100
// read_blob_chunk() returns only after reading the requested length;
101101
// otherwise it throws, so bytes_read is intentionally not used here.
102102
[[maybe_unused]] auto const bytes_read = read_blob_chunk(
103103
fp,
104104
path,
105-
reinterpret_cast<char*>(buffer + blob_header_size),
105+
std::next(buffer, blob_header_size),
106106
payload_capacity);
107107
}
108108
return {true, ""};
@@ -133,7 +133,7 @@ rdma_send_stream_base::buffer_fill_result rdma_socket_io::fill_blob_data_chunk(
133133
// read_blob_chunk() returns only after reading the requested length;
134134
// otherwise it throws, so bytes_read is intentionally not used here.
135135
[[maybe_unused]] auto const bytes_read =
136-
read_blob_chunk(fp, path, reinterpret_cast<char*>(buffer), capacity);
136+
read_blob_chunk(fp, path, buffer, capacity);
137137
return {true, ""};
138138
}
139139

src/limestone/replication/blob_send_utils.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <cerrno>
44
#include <cstdio>
55
#include <cstring>
6+
#include <iterator>
67
#include <limits>
78

89
#include <boost/filesystem.hpp>
@@ -69,11 +70,13 @@ void safe_close_blob_file(FILE* fp, char const* failure_message_prefix) {
6970
std::size_t read_blob_chunk(
7071
FILE* fp,
7172
boost::filesystem::path const& path,
72-
char* buffer,
73+
std::uint8_t* buffer,
7374
std::size_t length) {
7475
std::size_t total_read = 0;
7576
while (total_read < length) {
76-
std::size_t r = std::fread(buffer + total_read, 1, length - total_read, fp);
77+
using buffer_offset_type = std::iterator_traits<std::uint8_t*>::difference_type;
78+
auto const offset = static_cast<buffer_offset_type>(total_read);
79+
std::size_t r = std::fread(std::next(buffer, offset), 1, length - total_read, fp);
7780
if (r == 0) {
7881
int ec = errno;
7982
if (ec == EINTR) {

src/limestone/replication/blob_send_utils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ void safe_close_blob_file(FILE* fp, char const* failure_message_prefix);
5151
[[nodiscard]] std::size_t read_blob_chunk(
5252
FILE* fp,
5353
boost::filesystem::path const& path,
54-
char* buffer,
54+
std::uint8_t* buffer,
5555
std::size_t length);
5656

5757
} // namespace limestone::replication

src/limestone/replication/blob_socket_io.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ void blob_socket_io::send_blob(const blob_id_type blob_id) {
2727
send_uint64(blob_id);
2828
send_uint32(remaining);
2929

30-
std::vector<char> buffer(blob_buffer_size);
30+
std::vector<std::uint8_t> buffer(blob_buffer_size);
3131
while (remaining > 0) {
3232
std::size_t chunk = std::min(blob_buffer_size, static_cast<std::size_t>(remaining));
3333
std::size_t total_read = read_blob_chunk(opened.fp, opened.path, buffer.data(), chunk);
34-
get_out_stream().write(buffer.data(), static_cast<std::streamsize>(total_read));
34+
get_out_stream().write(
35+
reinterpret_cast<char const*>(buffer.data()), // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
36+
static_cast<std::streamsize>(total_read));
3537
remaining -= static_cast<uint32_t>(total_read);
3638
}
3739

test/limestone/replication/blob_send_utils_test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ TEST_F(blob_send_utils_test, read_blob_chunk_reads_exact_bytes) {
9999
opened_blob_file opened = open_blob_file_for_send(*datastore_, blob_id);
100100
ASSERT_NE(opened.fp, nullptr);
101101

102-
std::vector<char> buffer(6);
102+
std::vector<std::uint8_t> buffer(6);
103103
std::size_t bytes_read = read_blob_chunk(
104104
opened.fp, opened.path, buffer.data(), buffer.size());
105105
EXPECT_EQ(bytes_read, buffer.size());
@@ -116,7 +116,7 @@ TEST_F(blob_send_utils_test, read_blob_chunk_throws_on_unexpected_eof) {
116116
opened_blob_file opened = open_blob_file_for_send(*datastore_, blob_id);
117117
ASSERT_NE(opened.fp, nullptr);
118118

119-
std::vector<char> buffer(content.size() + 1);
119+
std::vector<std::uint8_t> buffer(content.size() + 1);
120120
EXPECT_THROW({
121121
[[maybe_unused]] std::size_t bytes_read =
122122
read_blob_chunk(opened.fp, opened.path, buffer.data(), buffer.size());

0 commit comments

Comments
 (0)