Add copying utility methods to Arena

benchmarks/bench_parser_comparison.cpp

@@ -76,13 +76,6 @@ private:
   Precondition current_precondition;
   Operation current_operation;
 
-  // Helper to store string in arena and return string_view
-  std::string_view store_string(const char *str, size_t length) {
-    char *stored = arena.allocate<char>(length);
-    std::memcpy(stored, str, length);
-    return std::string_view(stored, length);
-  }
-
 public:
   explicit CommitRequestArenaHandler()
       : preconditions(ArenaStlAllocator<Precondition>(&arena)),
@@ -109,7 +102,7 @@ public:
   bool RawNumber(const char *, rapidjson::SizeType, bool) { abort(); }
 
   bool String(const char *str, rapidjson::SizeType length, bool) {
-    std::string_view value = store_string(str, length);
+    std::string_view value = arena.copy_string({str, length});
 
     if (state == State::Root) {
       if (current_key == "request_id") {

src/arena.hpp

@@ -405,7 +405,6 @@ public:
    * ## Note:
    * This method only allocates memory - it does not construct objects.
    * Use placement new or other initialization methods as needed.
-   * TODO should this return a std::span<T> ?
    */
   template <typename T> T *allocate(uint32_t size) {
     static_assert(
@@ -427,6 +426,72 @@ public:
     return static_cast<T *>(ptr);
   }
 
+  /**
+   * @brief Allocate an array of type T and return it as a std::span<T>.
+   *
+   * This method provides bounds-safe allocation by returning a std::span
+   * that knows its size, improving safety over raw pointer allocation.
+   *
+   * @tparam T The type to allocate (must be trivially destructible)
+   * @param count The number of elements to allocate
+   * @return std::span<T> A span covering the allocated array
+   *
+   * ## Safety:
+   * The returned span is valid for the lifetime of the arena and until
+   * the next reset() call. The span provides bounds checking in debug
+   * builds and clear size information.
+   *
+   * ## Usage:
+   * ```cpp
+   * auto buffer = arena.allocate_span<char>(1024);
+   * auto strings = arena.allocate_span<std::string_view>(10);
+   * ```
+   *
+   * ## Note:
+   * Returns an empty span (nullptr, 0) if count is 0.
+   * This method only allocates memory - it does not construct objects.
+   */
+  template <typename T> std::span<T> allocate_span(uint32_t count) {
+    if (count == 0) {
+      return std::span<T>{};
+    }
+    return std::span<T>{allocate<T>(count), count};
+  }
+
+  /**
+   * @brief Copy a string into arena memory and return a string_view.
+   *
+   * This method provides a safe way to copy string data into arena-allocated
+   * memory, ensuring the data remains valid for the arena's lifetime.
+   *
+   * @param str The string to copy into arena memory
+   * @return std::string_view pointing to the arena-allocated copy
+   *
+   * ## Safety:
+   * The returned string_view is valid for the lifetime of the arena and until
+   * the next reset() call. The string data is guaranteed to be null-terminated
+   * only if the input string was null-terminated.
+   *
+   * ## Usage:
+   * ```cpp
+   * Arena arena;
+   * std::string_view copy = arena.copy_string("Hello World");
+   * std::string_view copy2 = arena.copy_string(some_string_view);
+   * ```
+   *
+   * ## Note:
+   * Returns an empty string_view if the input string is empty.
+   * This method allocates exactly str.size() bytes (no null terminator added).
+   */
+  std::string_view copy_string(std::string_view str) {
+    if (str.empty()) {
+      return std::string_view{};
+    }
+    char *copied = allocate<char>(str.size());
+    std::memcpy(copied, str.data(), str.size());
+    return std::string_view(copied, str.size());
+  }
+
   /**
    * @brief Reset the allocator to reuse the first block, freeing all others.
    *

src/commit_request.hpp

@@ -244,14 +244,7 @@ public:
    * @return String view pointing to arena-allocated memory
    */
   std::string_view copy_to_arena(std::string_view str) {
-    if (str.empty()) {
-      return {};
-    }
-
-    char *arena_str = arena_.allocate<char>(str.size());
-    std::memcpy(arena_str, str.data(), str.size());
-
-    return std::string_view(arena_str, str.size());
+    return arena_.copy_string(str);
   }
 
   /**

src/connection.hpp

@@ -44,7 +44,7 @@ struct MessageSender {
    *
    * Example usage:
    * ```cpp
-   * auto response_parts = std::span{arena.allocate<std::string_view>(2), 2};
+   * auto response_parts = arena.allocate_span<std::string_view>(2);
    * response_parts[0] = "HTTP/1.1 200 OK\r\n\r\n";
    * response_parts[1] = "Hello World";
    * conn.append_message(response_parts, std::move(arena));
@@ -121,7 +121,7 @@ struct Connection : MessageSender {
    * Example usage:
    * ```cpp
    * Arena arena;
-   * auto* parts = arena.allocate<std::string_view>(2);
+   * auto parts = arena.allocate_span<std::string_view>(2);
    * parts[0] = build_header(arena);
    * parts[1] = build_body(arena);
    * conn.append_message({parts, 2}, std::move(arena));

src/http_handler.cpp

@@ -389,9 +389,8 @@ void HttpHandler::handle_get_metrics(Connection &conn,
         "\r\n", "Connection: keep-alive\r\n", "\r\n");
   }
 
-  auto result = std::span<std::string_view>{
-      state.arena.allocate<std::string_view>(metrics_span.size() + 1),
-      metrics_span.size() + 1};
+  auto result =
+      state.arena.allocate_span<std::string_view>(metrics_span.size() + 1);
   auto out = result.begin();
   *out++ = headers;
   for (auto sv : metrics_span) {
@@ -443,7 +442,7 @@ void HttpHandler::send_response(MessageSender &conn, int status_code,
 
   const char *connection_header = close_connection ? "close" : "keep-alive";
 
-  auto response = std::span{response_arena.allocate<std::string_view>(1), 1};
+  auto response = response_arena.allocate_span<std::string_view>(1);
 
   response[0] =
       format(response_arena,
@@ -509,7 +508,7 @@ HttpHandler::format_response(int status_code, std::string_view content_type,
 
   const char *connection_header = close_connection ? "close" : "keep-alive";
 
-  auto response = std::span{response_arena.allocate<std::string_view>(1), 1};
+  auto response = response_arena.allocate_span<std::string_view>(1);
 
   response[0] =
       format(response_arena,
@@ -733,12 +732,9 @@ bool HttpHandler::process_sequence_batch(BatchType &batch) {
             if (!status_entry.status_request_id.empty()) {
               // Add request_id to banned list - store the string in arena and
               // use string_view
-              char *arena_chars = banned_request_arena.allocate<char>(
-                  status_entry.status_request_id.size());
-              std::memcpy(arena_chars, status_entry.status_request_id.data(),
-                          status_entry.status_request_id.size());
-              std::string_view request_id_view(
-                  arena_chars, status_entry.status_request_id.size());
+              std::string_view request_id_view =
+                  banned_request_arena.copy_string(
+                      status_entry.status_request_id);
               banned_request_ids.insert(request_id_view);
 
               // Update memory usage metric

src/metric.cpp

@@ -123,16 +123,6 @@ static void validate_or_abort(bool condition, const char *message,
   }
 }
 
-// Helper to copy a string into arena memory
-static std::string_view arena_copy_string(std::string_view str, Arena &arena) {
-  if (str.empty()) {
-    return std::string_view{};
-  }
-  char *copied = arena.allocate<char>(str.size());
-  std::memcpy(copied, str.data(), str.size());
-  return std::string_view(copied, str.size());
-}
-
 // Arena-based labels key for second level of map
 // Uses string_view containing labels in Prometheus text format
 struct LabelsKey {
@@ -149,8 +139,8 @@ struct LabelsKey {
       validate_or_abort(is_valid_label_value(value), "invalid label value",
                         value);
 
-      auto key_view = arena_copy_string(key, arena);
-      auto value_view = arena_copy_string(value, arena);
+      auto key_view = arena.copy_string(key);
+      auto value_view = arena.copy_string(value);
       labels.push_back({key_view, value_view});
     }
 
@@ -581,7 +571,7 @@ struct Metric {
     }
 
     // Not found - copy to global arena and intern
-    auto interned_text = arena_copy_string(text, get_global_arena());
+    auto interned_text = get_global_arena().copy_string(text);
     auto result = interned_set.emplace(interned_text);
     return *result.first;
   }
@@ -1461,12 +1451,12 @@ Family<Counter> create_counter(std::string_view name, std::string_view help) {
   std::unique_lock _{Metric::mutex};
   ++Metric::registration_version;
   auto &global_arena = Metric::get_global_arena();
-  auto name_view = arena_copy_string(name, global_arena);
+  auto name_view = global_arena.copy_string(name);
   auto &familyPtr = Metric::get_counter_families()[name_view];
   if (!familyPtr) {
     familyPtr = global_arena.construct<Family<Counter>::State>(global_arena);
     familyPtr->name = name_view;
-    familyPtr->help = arena_copy_string(help, global_arena);
+    familyPtr->help = global_arena.copy_string(help);
   } else {
     validate_or_abort(
         familyPtr->help == help,
@@ -1483,13 +1473,13 @@ Family<Gauge> create_gauge(std::string_view name, std::string_view help) {
   std::unique_lock _{Metric::mutex};
   ++Metric::registration_version;
   auto &global_arena = Metric::get_global_arena();
-  auto name_view = arena_copy_string(name, global_arena);
+  auto name_view = global_arena.copy_string(name);
   auto &familyPtr = Metric::get_gauge_families()[name_view];
   if (!familyPtr) {
     // Family<T>::State instances use Arena::Ptr for automatic cleanup
     familyPtr = global_arena.construct<Family<Gauge>::State>(global_arena);
     familyPtr->name = name_view;
-    familyPtr->help = arena_copy_string(help, global_arena);
+    familyPtr->help = global_arena.copy_string(help);
   } else {
     validate_or_abort(
         familyPtr->help == help,
@@ -1507,13 +1497,13 @@ Family<Histogram> create_histogram(std::string_view name, std::string_view help,
   std::unique_lock _{Metric::mutex};
   ++Metric::registration_version;
   auto &global_arena = Metric::get_global_arena();
-  auto name_view = arena_copy_string(name, global_arena);
+  auto name_view = global_arena.copy_string(name);
   auto &family_ptr = Metric::get_histogram_families()[name_view];
   if (!family_ptr) {
     // Family<T>::State instances use Arena::Ptr for automatic cleanup
     family_ptr = global_arena.construct<Family<Histogram>::State>(global_arena);
     family_ptr->name = name_view;
-    family_ptr->help = arena_copy_string(help, global_arena);
+    family_ptr->help = global_arena.copy_string(help);
 
     // DESIGN: Prometheus-compatible histogram buckets
     // Convert to vector for sorting

style.md

@@ -396,6 +396,27 @@ operations = {};  // Default construct - clear() won't work correctly
 arena.reset();  // Reset arena memory
 ```
 
+### Arena String Copying
+
+- **Always use `Arena::copy_string()`** for copying string data into arena memory
+- **Avoid manual allocation and memcpy** for string copying
+- **Use `Arena::allocate_span<T>()`** for array allocations instead of manual span construction
+
+```cpp
+// Preferred - unified arena methods
+std::string_view copy = arena.copy_string(original_string);
+auto buffer = arena.allocate_span<char>(1024);
+auto strings = arena.allocate_span<std::string_view>(count);
+
+// Avoid - manual allocation and copying
+char *copied = arena.allocate<char>(str.size());
+std::memcpy(copied, str.data(), str.size());
+std::string_view copy(copied, str.size());
+
+// Avoid - manual span construction
+auto span = std::span{arena.allocate<std::string_view>(count), count};
+```
+
 ### Resource Management
 
 - **RAII** everywhere - constructors acquire, destructors release

tests/test_metric.cpp

@@ -585,32 +585,6 @@ TEST_CASE("thread counter cleanup bug") {
   }
 }
 
-TEST_CASE("error conditions") {
-  SUBCASE("counter negative increment") {
-    auto counter_family = metric::create_counter("error_counter", "Error test");
-    auto counter = counter_family.create({});
-
-    // This should abort in debug builds due to validation
-    // In release builds, behavior is undefined
-    // counter.inc(-1.0); // Would abort
-  }
-
-  SUBCASE("invalid metric names") {
-    // These should abort due to validation
-    // auto bad_counter = metric::create_counter("123invalid", "help"); // Would
-    // abort auto bad_gauge = metric::create_gauge("invalid-name", "help");   //
-    // Would abort
-  }
-
-  SUBCASE("invalid label keys") {
-    auto counter_family = metric::create_counter("valid_name", "help");
-
-    // This should abort due to label validation
-    // auto counter = counter_family.create({{"123invalid", "value"}}); // Would
-    // abort
-  }
-}
-
 TEST_CASE("memory management") {
   SUBCASE("arena allocation in render") {
     Arena arena;

tests/test_server.cpp

@@ -8,24 +8,14 @@
 #include <string_view>
 #include <thread>
 
-// Helper to copy a string into arena memory
-static std::string_view arena_copy_string(std::string_view str, Arena &arena) {
-  if (str.empty()) {
-    return std::string_view{};
-  }
-  char *copied = arena.allocate<char>(str.size());
-  std::memcpy(copied, str.data(), str.size());
-  return std::string_view(copied, str.size());
-}
-
 struct EchoHandler : ConnectionHandler {
   Arena arena;
   std::span<std::string_view> reply;
   WeakRef<MessageSender> wconn;
   std::latch done{1};
   void on_data_arrived(std::string_view data, Connection &conn) override {
-    reply = std::span{arena.allocate<std::string_view>(1), 1};
-    reply[0] = arena_copy_string(data, arena);
+    reply = arena.allocate_span<std::string_view>(1);
+    reply[0] = arena.copy_string(data);
     wconn = conn.get_weak_ref();
     CHECK(wconn.lock());
     done.count_down();