Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 0 additions & 33 deletions .vscode/launch.json

This file was deleted.

82 changes: 19 additions & 63 deletions api/include/opentelemetry/context/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

#pragma once

#include <cstring>
#include <string>
#include <utility>

#include "opentelemetry/context/context_value.h"
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ABI break

: 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_;
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid and better, but ABI break

{
Context context = Context(key, value);
Context context(key, value);
context.head_->next_ = head_;
return context;
}
Expand All @@ -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{};
Expand All @@ -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_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ABI break


nostd::shared_ptr<DataList> next_{nullptr};

size_t key_length_ = 0UL;

ContextValue value_;

DataList() = default;
Expand All @@ -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
Expand Down
35 changes: 20 additions & 15 deletions api/include/opentelemetry/nostd/shared_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#endif

#if !defined(OPENTELEMETRY_HAVE_STD_SHARED_PTR)
# include <cstdlib>
# include <memory>
# include <utility>

Expand All @@ -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
{
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid and better, but ABI break.

{
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_)};
}
Expand All @@ -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)]{};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ABI break

};

static_assert(alignof(shared_ptr_wrapper) <= kAlignment, "Placement buffer not properly aligned");

public:
Expand Down Expand Up @@ -105,20 +102,24 @@ class shared_ptr

shared_ptr(unique_ptr<T> &&other) noexcept
{
std::shared_ptr<T> ptr_(other.release());
std::shared_ptr<T> ptr_(std::move(other));
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using std::move() on a nostd::unique_ptr will not work correctly. The original code with other.release() was correct. nostd::unique_ptr may not have a move constructor that works with std::shared_ptr, but it does have a release() method that returns the raw pointer. This change will likely cause compilation errors or incorrect behavior.

Suggested change
std::shared_ptr<T> ptr_(std::move(other));
std::shared_ptr<T> ptr_(other.release());

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@Reneg973 Reneg973 Oct 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this!
According to cppref(constructor 13) this constructor exists. Additionally nostd::unique_ptr implements move operations perfectly.
Also, if this would be true, then code like:

nostd::unique_ptr<T> Foo() { return new T; }
std::shared_ptr<T> ptr(Foo());

would not work.

PS: GPT-5 says everything is fine with this code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, std::shared_ptr should not recognize nostd::unique_ptr and cannot be constructed from its rvalue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, std::shared_ptr should not recognize nostd::unique_ptr and cannot be constructed from its rvalue.

Perfect, I was blind and naive. Thought I saw a test for it, but didn't. After writing one I directly had compiler error.

new (buffer_.data) shared_ptr_wrapper{std::move(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;
}
Comment on lines +119 to +122
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is valid, please post a separate PR for this fix,

wrapper().~shared_ptr_wrapper();
other.wrapper().MoveTo(buffer_);
return *this;
Expand All @@ -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;
Expand Down
Loading