Context and shared ptr improvements

api/include/opentelemetry/context/context.h

@@ -3,7 +3,7 @@
 
 #pragma once
 
-#include <cstring>
+#include <string>
 #include <utility>
 
 #include "opentelemetry/context/context_value.h"
@@ -34,18 +34,18 @@ class Context
 
   // Creates a context object from a key and value, this will
   // hold a shared_ptr to the head of the DataList linked list
-  Context(nostd::string_view key, ContextValue value) noexcept
+  Context(nostd::string_view key, const ContextValue &value) noexcept
       : head_{nostd::shared_ptr<DataList>{new DataList(key, value)}}
   {}
 
   // Accepts a new iterable and then returns a new context that
   // contains the new key and value data. It attaches the
   // exisiting list to the end of the new list.
   template <class T>
-  Context SetValues(T &values) noexcept
+  Context SetValues(T &values) const noexcept
   {
-    Context context                  = Context(values);
-    nostd::shared_ptr<DataList> last = context.head_;
+    Context context(values);
+    auto last = context.head_;
     while (last->next_ != nullptr)
     {
       last = last->next_;
@@ -57,9 +57,9 @@ class Context
   // Accepts a new iterable and then returns a new context that
   // contains the new key and value data. It attaches the
   // exisiting list to the end of the new list.
-  Context SetValue(nostd::string_view key, ContextValue value) noexcept
+  Context SetValue(nostd::string_view key, const ContextValue &value) const noexcept
   {
-    Context context      = Context(key, value);
+    Context context(key, value);
     context.head_->next_ = head_;
     return context;
   }
@@ -69,12 +69,9 @@ class Context
   {
     for (DataList *data = head_.get(); data != nullptr; data = data->next_.get())
     {
-      if (key.size() == data->key_length_)
+      if (key == data->key_)
       {
-        if (std::memcmp(key.data(), data->key_, data->key_length_) == 0)
-        {
-          return data->value_;
-        }
+        return data->value_;
       }
     }
     return ContextValue{};
@@ -92,12 +89,10 @@ class Context
   // A linked list to contain the keys and values of this context node
   struct DataList
   {
-    char *key_ = nullptr;
+    std::string key_;
 
     nostd::shared_ptr<DataList> next_{nullptr};
 
-    size_t key_length_ = 0UL;
-
     ContextValue value_;
 
     DataList() = default;
@@ -106,62 +101,23 @@ class Context
     template <class T>
     DataList(const T &keys_and_vals)
     {
-      bool first = true;
+      auto iter = std::begin(keys_and_vals);
+      if (iter == std::end(keys_and_vals))
+        return;
       auto *node = this;
-      for (auto &iter : keys_and_vals)
+      *node      = DataList(iter->first, iter->second);
+      for (++iter; iter != std::end(keys_and_vals); ++iter)
       {
-        if (first)
-        {
-          *node = DataList(iter.first, iter.second);
-          first = false;
-        }
-        else
-        {
-          node->next_ = nostd::shared_ptr<DataList>(new DataList(iter.first, iter.second));
-          node        = node->next_.get();
-        }
+        node->next_ = nostd::shared_ptr<DataList>(new DataList(iter->first, iter->second));
+        node        = node->next_.get();
       }
     }
 
     // Builds a data list with just a key and value, so it will just be the head
     // and returns that head.
     DataList(nostd::string_view key, const ContextValue &value)
-    {
-      key_        = new char[key.size()];
-      key_length_ = key.size();
-      std::memcpy(key_, key.data(), key.size() * sizeof(char));
-      next_  = nostd::shared_ptr<DataList>{nullptr};
-      value_ = value;
-    }
-
-    DataList(const DataList &other)
-        : key_(new char[other.key_length_]),
-          next_(other.next_),
-          key_length_(other.key_length_),
-          value_(other.value_)
-    {
-      std::memcpy(key_, other.key_, other.key_length_ * sizeof(char));
-    }
-
-    DataList &operator=(DataList &&other) noexcept
-    {
-      key_length_ = other.key_length_;
-      value_      = std::move(other.value_);
-      next_       = std::move(other.next_);
-
-      key_       = other.key_;
-      other.key_ = nullptr;
-
-      return *this;
-    }
-
-    ~DataList()
-    {
-      if (key_ != nullptr)
-      {
-        delete[] key_;
-      }
-    }
+        : key_(key.begin(), key.end()), value_(value)
+    {}
   };
 
   // Head of the list which holds the keys and values of this context

api/include/opentelemetry/nostd/shared_ptr.h

@@ -11,7 +11,6 @@
 #endif
 
 #if !defined(OPENTELEMETRY_HAVE_STD_SHARED_PTR)
-#  include <cstdlib>
 #  include <memory>
 #  include <utility>
 
@@ -32,13 +31,9 @@ class shared_ptr
   using pointer      = element_type *;
 
 private:
-  static constexpr size_t kMaxSize   = 32;
   static constexpr size_t kAlignment = 8;
 
-  struct alignas(kAlignment) PlacementBuffer
-  {
-    char data[kMaxSize]{};
-  };
+  struct alignas(kAlignment) PlacementBuffer;  // fwd.
 
   class shared_ptr_wrapper
   {
@@ -47,14 +42,12 @@ class shared_ptr
 
     shared_ptr_wrapper(std::shared_ptr<T> &&ptr) noexcept : ptr_{std::move(ptr)} {}
 
-    virtual ~shared_ptr_wrapper() {}
-
-    virtual void CopyTo(PlacementBuffer &buffer) const noexcept
+    void CopyTo(PlacementBuffer &buffer) const noexcept
     {
       new (buffer.data) shared_ptr_wrapper{*this};
     }
 
-    virtual void MoveTo(PlacementBuffer &buffer) noexcept
+    void MoveTo(PlacementBuffer &buffer) noexcept
     {
       new (buffer.data) shared_ptr_wrapper{std::move(this->ptr_)};
     }
@@ -67,15 +60,19 @@ class shared_ptr
       new (buffer.data) other_shared_ptr_wrapper{std::move(this->ptr_)};
     }
 
-    virtual pointer Get() const noexcept { return ptr_.get(); }
+    pointer Get() const noexcept { return ptr_.get(); }
 
-    virtual void Reset() noexcept { ptr_.reset(); }
+    void Reset() noexcept { ptr_.reset(); }
 
   private:
     std::shared_ptr<T> ptr_;
   };
 
-  static_assert(sizeof(shared_ptr_wrapper) <= kMaxSize, "Placement buffer is too small");
+  struct alignas(kAlignment) PlacementBuffer
+  {
+    char data[sizeof(shared_ptr_wrapper)]{};
+  };
+
   static_assert(alignof(shared_ptr_wrapper) <= kAlignment, "Placement buffer not properly aligned");
 
 public:
@@ -111,14 +108,18 @@ class shared_ptr
 
   shared_ptr(std::unique_ptr<T> &&other) noexcept
   {
-    std::shared_ptr<T> ptr_(other.release());
+    std::shared_ptr<T> ptr_(std::move(other));
     new (buffer_.data) shared_ptr_wrapper{std::move(ptr_)};
   }
 
   ~shared_ptr() { wrapper().~shared_ptr_wrapper(); }
 
   shared_ptr &operator=(shared_ptr &&other) noexcept
   {
+    if (this == &other)
+    {
+      return *this;
+    }
     wrapper().~shared_ptr_wrapper();
     other.wrapper().MoveTo(buffer_);
     return *this;
@@ -132,6 +133,10 @@ class shared_ptr
 
   shared_ptr &operator=(const shared_ptr &other) noexcept
   {
+    if (this == &other)
+    {
+      return *this;
+    }
     wrapper().~shared_ptr_wrapper();
     other.wrapper().CopyTo(buffer_);
     return *this;

api/test/nostd/shared_ptr_test.cc

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "opentelemetry/nostd/shared_ptr.h"
+#include "opentelemetry/nostd/unique_ptr.h"
 
 using opentelemetry::nostd::shared_ptr;
 
@@ -96,6 +97,15 @@ TEST(SharedPtrTest, MoveConstructionFromStdSharedPtr)
   EXPECT_EQ(ptr2.get(), value);
 }
 
+TEST(SharedPtrTest, MoveConstructionFromNoStdUniquePtr)
+{
+  opentelemetry::v1::nostd::unique_ptr<int> value(new int{123});
+  auto p = value.get();
+  shared_ptr<int> ptr{std::move(value)};
+  EXPECT_EQ(value.get(), nullptr);  // NOLINT
+  EXPECT_EQ(ptr.get(), p);
+}
+
 TEST(SharedPtrTest, Destruction)
 {
   bool was_destructed;