Use const Coroutine pointers

MODULE.bazel

@@ -12,13 +12,16 @@ bazel_dep(name = "googletest", version = "1.15.2", repo_name = "com_google_googl
 # Coroutines
 http_archive(
     name = "coroutines",
-    integrity = "sha256-PhOYq1eE8Q8UOhzDHq2+rafTU4VTt9fZ0ZZyNE+hWb4=",
-    strip_prefix = "co-2.1.0",
-    urls = ["https://github.com/dallison/co/archive/refs/tags/2.1.0.tar.gz"],
+    # integrity = "sha256-tRZ5YigZhxVJDBENpQwqL1Bch1VPuV+tgKSK+/72aTU=",
+    sha256 = "d38c2a2480f016ed44c8460b9b9c6436445211bf14b6b0947e7577ccb29ca9d4",
+    strip_prefix = "co-562a5a335570c8ab70d8d36f3e6d7e85ccccfd10",
+    # urls = ["https://github.com/dallison/co/archive/refs/tags/2.1.8.tar.gz"],
+    urls = ["https://github.com/dallison/co/archive/562a5a335570c8ab70d8d36f3e6d7e85ccccfd10.tar.gz"],
 )
 # For local debugging of co coroutine library.
 # bazel_dep(name = "coroutines")
 # local_path_override(
-#    module_name = "coroutines",
-#    path = "../co",
+#   module_name = "coroutines",
+#   path = "../co",
 # )
+

toolbelt/BUILD.bazel

@@ -12,6 +12,7 @@ cc_library(
         "sockets.cc",
         "table.cc",
         "triggerfd.cc",
+        "stacktrace.cc",
     ],
     hdrs = [
         "bitset.h",
@@ -26,14 +27,15 @@ cc_library(
         "sockets.h",
         "table.h",
         "triggerfd.h",
+        "stacktrace.h",
     ],
     deps = [
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/types:span",
-        "@coroutines//:co",
+        "@coroutines//co",
     ],
 )
 
@@ -63,7 +65,19 @@ cc_test(
         "@com_google_googletest//:gtest_main",
     ],
 )
-
+cc_test(
+    name = "stacktrace_test",
+    size = "small",
+    srcs = ["stacktrace_test.cc"],
+    deps = [
+        ":toolbelt",
+        "@com_google_absl//absl/hash:hash_testing",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings:str_format",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
 cc_test(
     name = "pipe_test",
     size = "small",
@@ -86,6 +100,7 @@ cc_test(
         ":toolbelt",
         "@com_google_absl//absl/hash:hash_testing",
         "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:status_matchers",
         "@com_google_absl//absl/status:statusor",
         "@com_google_googletest//:gtest_main",
     ],

toolbelt/fd.cc

@@ -16,7 +16,7 @@ void CloseAllFds(std::function<bool(int)> predicate) {
 }
 
 absl::StatusOr<ssize_t> FileDescriptor::Read(void *buffer, size_t length,
-                                             co::Coroutine *c) {
+                                             const co::Coroutine *c) {
   char *buf = reinterpret_cast<char *>(buffer);
   size_t total = 0;
   while (total < length) {
@@ -58,7 +58,7 @@ absl::StatusOr<ssize_t> FileDescriptor::Read(void *buffer, size_t length,
 }
 
 absl::StatusOr<ssize_t> FileDescriptor::Write(const void *buffer, size_t length,
-                                              co::Coroutine *c) {
+                                              const co::Coroutine *c) {
   const char *buf = reinterpret_cast<const char *>(buffer);
 
   size_t total = 0;

toolbelt/fd.h

@@ -18,7 +18,7 @@
 #include <sys/resource.h>
 #include <sys/stat.h>
 #include <unistd.h>
-#include "coroutine.h"
+#include "co/coroutine.h"
 
 namespace toolbelt {
 
@@ -56,7 +56,8 @@ class FileDescriptor {
   FileDescriptor() = default;
   // FileDescriptor initialize with an OS fd.  Takes ownership
   // of the fd and will close it when all references go away.
-  explicit FileDescriptor(int fd) : data_(std::make_shared<SharedData>(fd)) {}
+  // If owned is false, the fd will not be closed when all references go away.
+  explicit FileDescriptor(int fd, bool owned = true) : data_(std::make_shared<SharedData>(fd, owned)) {}
 
   // Copy constructor, increments reference on shared data.  Very cheap.
   FileDescriptor(const FileDescriptor &f) : data_(f.data_) {}
@@ -119,13 +120,13 @@ class FileDescriptor {
   // Sets the OS fd.  If it's the same as the underlying OS fd, there is
   // no effect (that's not another reference to it).  Allocates new
   // shared data for the fd.
-  void SetFd(int fd) {
+  void SetFd(int fd, bool owned = true) {
     if (Fd() == fd) {
       // SetFd with same fd.  This isn't another reference to the
       // fd.
       return;
     }
-    data_ = std::make_shared<SharedData>(fd);
+    data_ = std::make_shared<SharedData>(fd, owned);
   }
 
   void Reset() { Close(); }
@@ -187,23 +188,27 @@ class FileDescriptor {
     return absl::OkStatus();
   }
 
-  absl::StatusOr<ssize_t> Read(void* buffer, size_t length, co::Coroutine* c = nullptr);
+  absl::StatusOr<ssize_t> Read(void* buffer, size_t length, const co::Coroutine* c = nullptr);
   absl::StatusOr<ssize_t> Write(const void* buffer, size_t length,
-                                      co::Coroutine* c = nullptr);
+                                      const co::Coroutine* c = nullptr);
 private:
   // Reference counted OS fd, shared among all FileDescriptors with the
   // same OS fd, provided you don't create two FileDescriptors with the
   // same OS fd (that would be a mistake but there's no way to stop it).
   struct SharedData {
     SharedData() = default;
-    SharedData(int f) : fd(f) {}
+    SharedData(int f, bool o) : fd(f), owned(o) {}
     ~SharedData() {
       if (fd != -1) {
+        if (!owned) {
+          return;
+        }
         ::close(fd);
       }
     }
     int fd = -1; // OS file descriptor.
     bool nonblocking = false;
+    bool owned = true;
   };
 
   // The actual shared data.  If nullptr the FileDescriptor is invalid.

toolbelt/fd_test.cc

@@ -173,3 +173,26 @@ TEST(FdTest, Reset) {
   int e = fstat(f, &st);
   ASSERT_EQ(-1, e);
 }
+
+TEST(FdTest, CreateUnowned) {
+  int f = dup(1);
+  {
+    FileDescriptor fd(f, false);
+    ASSERT_TRUE(fd.Valid());
+    ASSERT_EQ(f, fd.Fd());
+    ASSERT_EQ(1, fd.RefCount());
+  }
+  // Fd should still be open.
+  ASSERT_EQ(0, fcntl(f, F_GETFD));
+
+  // Now take ownership of f.
+  {
+    FileDescriptor fd(f, true);
+    ASSERT_TRUE(fd.Valid());
+    ASSERT_EQ(f, fd.Fd());
+    ASSERT_EQ(1, fd.RefCount());
+  }
+  // Will be closed.
+  ASSERT_EQ(-1, fcntl(f, F_GETFD));
+  ASSERT_EQ(EBADF, errno);
+}

toolbelt/logging.cc

@@ -7,6 +7,7 @@
 #include "clock.h"
 #include <cstdio>
 #include <inttypes.h>
+#include <termios.h>
 
 namespace toolbelt {
 
@@ -148,10 +149,19 @@ void Logger::VLog(LogLevel level, const char *fmt, va_list ap) {
   if (level < min_level_) {
     return;
   }
+#if defined(__clang__)
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wformat-nonliteral"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
+#endif
   size_t n = vsnprintf(buffer_, sizeof(buffer_), fmt, ap);
+#if defined(__clang__)
 #pragma clang diagnostic pop
+#elif defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
 
   // Strip final \n if present.  Refactoring from printf can leave
   // this in place.

toolbelt/logging.h

@@ -10,7 +10,6 @@
 #include <stdarg.h>
 #include <string>
 #include <sys/ioctl.h>
-#include <termios.h>
 #include <unistd.h>
 #include <cstdint>
 

toolbelt/payload_buffer.cc

@@ -5,7 +5,7 @@
 namespace toolbelt {
 static constexpr struct BitmapRunInfo {
   int num;
-  int size;
+  uint32_t size;
 } bitmp_run_infos[kNumBitmapRuns] = {
     {kRunSize1, kBitmapRunSize1},
     {kRunSize2, kBitmapRunSize2},
@@ -162,7 +162,7 @@ void PayloadBuffer::Dump(std::ostream &os) {
   os << "  free_list: " << free_list << " " << ToAddress(free_list)
      << std::endl;
   os << "  message: " << message << " " << ToAddress(message) << std::endl;
-  for (int i = 0; i < kNumBitmapRuns; i++) {
+  for (size_t i = 0; i < kNumBitmapRuns; i++) {
     os << "  bitmaps[" << i << "]: " << bitmaps[i] << " "
        << ToAddress(bitmaps[i]) << std::endl;
   }
@@ -576,9 +576,9 @@ void *PayloadBuffer::Realloc(PayloadBuffer **buffer, void *p, uint32_t n,
                            (n & kBitmapRunSizeMask);
 
         *len_ptr = encoded_size;
-        if (clear && n > decoded_length) {
+        if (clear && n > static_cast<uint32_t>(decoded_length)) {
           memset(reinterpret_cast<char *>(p) + decoded_length, 0,
-                 n - decoded_length);
+                 n - static_cast<uint32_t>(decoded_length));
         }
         return p;
       }
@@ -589,7 +589,7 @@ void *PayloadBuffer::Realloc(PayloadBuffer **buffer, void *p, uint32_t n,
         return NULL;
       }
       memcpy(newp, p, decoded_length);
-      if (clear && n > decoded_length) {
+      if (clear && n > static_cast<uint32_t>(decoded_length)) {
         memset(reinterpret_cast<char *>(newp) + decoded_length, 0,
                n - decoded_length);
       }
@@ -629,8 +629,8 @@ void *PayloadBuffer::Realloc(PayloadBuffer **buffer, void *p, uint32_t n,
       int diff = n - orig_length;
       if (alloc_addr + orig_length == free_addr) {
         // There is a free block above.  See if has enough space.
-        if (free_block->length > diff) {
-          ssize_t freelen = free_block->length - diff;
+        if (free_block->length > static_cast<uint32_t>(diff)) {
+          uint32_t freelen = free_block->length - static_cast<uint32_t>(diff);
           if (freelen > sizeof(FreeBlockHeader)) {
             (*buffer)->ExpandIntoFreeBlockAbove(free_block, n, diff, freelen,
                                                 len_ptr, next_ptr, clear);
@@ -642,7 +642,7 @@ void *PayloadBuffer::Realloc(PayloadBuffer **buffer, void *p, uint32_t n,
       if (prev != NULL) {
         uintptr_t prev_addr = (uintptr_t)prev;
         if (prev_addr + prev->length == (uintptr_t)alloc_block &&
-            prev->length >= diff) {
+            prev->length >= static_cast<uint32_t>(diff)) {
           // Previous free block is adjacent and has enough space in it.
           // Use start of new block as new address and place FreeBlockHeader
           // at newly free part.

toolbelt/payload_buffer.h

@@ -144,7 +144,7 @@ struct PayloadBuffer {
   PayloadBuffer(uint32_t size, bool bitmap_allocator = true)
       : magic(kFixedBufferMagic | (bitmap_allocator ? kBitMapFlag : 0)),
         message(0), hwm(0), full_size(size), metadata(0) {
-    for (int i = 0; i < kNumBitmapRuns; i++) {
+    for (size_t i = 0; i < kNumBitmapRuns; i++) {
       bitmaps[i] = 0;
     }
     InitFreeList();
@@ -163,7 +163,7 @@ struct PayloadBuffer {
   PayloadBuffer(uint32_t initial_size, Resizer r, bool bitmap_allocator = true)
       : magic(kMovableBufferMagic | (bitmap_allocator ? kBitMapFlag : 0)),
         message(0), hwm(0), full_size(initial_size), metadata(0) {
-    for (int i = 0; i < kNumBitmapRuns; i++) {
+    for (size_t i = 0; i < kNumBitmapRuns; i++) {
       bitmaps[i] = 0;
     }
     InitFreeList();

toolbelt/payload_buffer_test.cc

@@ -1,6 +1,7 @@
 #include "toolbelt/clock.h"
 #include "toolbelt/hexdump.h"
 #include "toolbelt/payload_buffer.h"
+#include <cstddef>
 #include <gtest/gtest.h>
 #include <sstream>
 
@@ -152,13 +153,13 @@ TEST(BufferTest, SmallBlockAllocFree) {
     blocks.push_back(addr);
   }
   // Free every 5th block.
-  for (int i = 0; i < blocks.size(); i++) {
+  for (size_t i = 0; i < blocks.size(); i++) {
     if (i % 5 == 0) {
       pb->Free(blocks[i]);
     }
   }
   // Now allocate every 5th block again.
-  for (int i = 0; i < blocks.size(); i++) {
+  for (size_t i = 0; i < blocks.size(); i++) {
     if (i % 5 == 0) {
       size_t size = sizes[i % sizes.size()];
       void *addr = PayloadBuffer::Allocate(&pb, size);
@@ -314,7 +315,7 @@ TEST(BufferTest, TypicalPerformance) {
         small_blocks.push_back(addr);
       }
       // Free some of the blocks.
-      for (int i = prev_size; i < small_blocks.size(); i++) {
+      for (size_t i = prev_size; i < small_blocks.size(); i++) {
         if (i % 8 == 0) {
           continue;
         }
@@ -341,7 +342,7 @@ TEST(BufferTest, TypicalPerformance) {
         large_blocks.push_back(addr);
       }
       // Free some of the blocks.
-      for (int i = prev_size; i < large_blocks.size(); i++) {
+      for (size_t i = prev_size; i < large_blocks.size(); i++) {
         if (i % 8 == 0) {
           continue;
         }