[Code health] Perform cppcheck cleanup (#3150)

.github/workflows/cppcheck.yml

@@ -56,7 +56,14 @@ jobs:
       - name: Count warnings
         run: |
           set +e
-          COUNT=`grep -c -E "\[.+\]" cppcheck.log`
-          echo "cppcheck reported ${COUNT} warning(s)"
-          # TODO: uncomment to enforce failing the build
-          # if [ $COUNT -ne 0 ] ; then exit 1 ; fi
+          readonly WARNING_COUNT=`grep -c -E "\[.+\]" cppcheck.log`
+          echo "cppcheck reported ${WARNING_COUNT} warning(s)"
+          # Acceptable limit, to decrease over time down to 0
+          readonly WARNING_LIMIT=10
+          # FAIL the build if WARNING_COUNT > WARNING_LIMIT
+          if [ $WARNING_COUNT -gt $WARNING_LIMIT ] ; then
+            exit 1
+          # WARN in annotations if WARNING_COUNT > 0
+          elif [ $WARNING_COUNT -gt 0 ] ; then
+            echo "::warning::cppcheck reported ${WARNING_COUNT} warning(s)"
+          fi

api/include/opentelemetry/baggage/baggage_context.h

@@ -27,7 +27,7 @@ inline nostd::shared_ptr<Baggage> GetBaggage(const context::Context &context) no
 }
 
 inline context::Context SetBaggage(context::Context &context,
-                                   nostd::shared_ptr<Baggage> baggage) noexcept
+                                   const nostd::shared_ptr<Baggage> &baggage) noexcept
 {
   return context.SetValue(kBaggageHeader, baggage);
 }

api/include/opentelemetry/context/context.h

@@ -29,16 +29,14 @@ class Context
   // hold a shared_ptr to the head of the DataList linked list
   template <class T>
   Context(const T &keys_and_values) noexcept
-  {
-    head_ = nostd::shared_ptr<DataList>{new DataList(keys_and_values)};
-  }
+      : head_{nostd::shared_ptr<DataList>{new DataList(keys_and_values)}}
+  {}
 
   // 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
-  {
-    head_ = nostd::shared_ptr<DataList>{new DataList(key, value)};
-  }
+      : 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
@@ -92,22 +90,21 @@ class Context
 
 private:
   // A linked list to contain the keys and values of this context node
-  class DataList
+  struct DataList
   {
-  public:
-    char *key_;
+    char *key_ = nullptr;
 
-    nostd::shared_ptr<DataList> next_;
+    nostd::shared_ptr<DataList> next_{nullptr};
 
-    size_t key_length_;
+    size_t key_length_ = 0UL;
 
     ContextValue value_;
 
-    DataList() { next_ = nullptr; }
+    DataList() = default;
 
     // Builds a data list off of a key and value iterable and returns the head
     template <class T>
-    DataList(const T &keys_and_vals) : key_{nullptr}, next_(nostd::shared_ptr<DataList>{nullptr})
+    DataList(const T &keys_and_vals)
     {
       bool first = true;
       auto *node = this;
@@ -132,9 +129,18 @@ class Context
     {
       key_        = new char[key.size()];
       key_length_ = key.size();
-      memcpy(key_, key.data(), key.size() * sizeof(char));
-      value_ = value;
+      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

api/include/opentelemetry/context/propagation/global_propagator.h

@@ -32,7 +32,7 @@ class OPENTELEMETRY_EXPORT GlobalTextMapPropagator
     return nostd::shared_ptr<TextMapPropagator>(GetPropagator());
   }
 
-  static void SetGlobalPropagator(nostd::shared_ptr<TextMapPropagator> prop) noexcept
+  static void SetGlobalPropagator(const nostd::shared_ptr<TextMapPropagator> &prop) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetPropagator() = prop;

api/include/opentelemetry/context/runtime_context.h

@@ -152,7 +152,8 @@ class OPENTELEMETRY_EXPORT RuntimeContext
    *
    * @param storage a custom runtime context storage
    */
-  static void SetRuntimeContextStorage(nostd::shared_ptr<RuntimeContextStorage> storage) noexcept
+  static void SetRuntimeContextStorage(
+      const nostd::shared_ptr<RuntimeContextStorage> &storage) noexcept
   {
     GetStorage() = storage;
   }

api/include/opentelemetry/logs/event_id.h

@@ -19,9 +19,9 @@ namespace logs
 class EventId
 {
 public:
-  EventId(int64_t id, nostd::string_view name) noexcept : id_{id}
+  EventId(int64_t id, nostd::string_view name) noexcept
+      : id_{id}, name_{nostd::unique_ptr<char[]>{new char[name.length() + 1]}}
   {
-    name_ = nostd::unique_ptr<char[]>{new char[name.length() + 1]};
     std::copy(name.begin(), name.end(), name_.get());
     name_.get()[name.length()] = 0;
   }

api/include/opentelemetry/logs/provider.h

@@ -39,7 +39,7 @@ class OPENTELEMETRY_EXPORT Provider
   /**
    * Changes the singleton LoggerProvider.
    */
-  static void SetLoggerProvider(nostd::shared_ptr<LoggerProvider> tp) noexcept
+  static void SetLoggerProvider(const nostd::shared_ptr<LoggerProvider> &tp) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetProvider() = tp;
@@ -60,7 +60,7 @@ class OPENTELEMETRY_EXPORT Provider
   /**
    * Changes the singleton EventLoggerProvider.
    */
-  static void SetEventLoggerProvider(nostd::shared_ptr<EventLoggerProvider> tp) noexcept
+  static void SetEventLoggerProvider(const nostd::shared_ptr<EventLoggerProvider> &tp) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetEventProvider() = tp;

api/include/opentelemetry/metrics/provider.h

@@ -38,7 +38,7 @@ class Provider
   /**
    * Changes the singleton MeterProvider.
    */
-  static void SetMeterProvider(nostd::shared_ptr<MeterProvider> tp) noexcept
+  static void SetMeterProvider(const nostd::shared_ptr<MeterProvider> &tp) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetProvider() = tp;

api/include/opentelemetry/nostd/internal/absl/types/internal/variant.h

@@ -221,6 +221,7 @@ template <class ReturnType, class FunctionObject, std::size_t... BoundIndices>
 struct MakeVisitationMatrix<ReturnType, FunctionObject, index_sequence<>,
                             index_sequence<BoundIndices...>> {
   using ResultType = ReturnType (*)(FunctionObject&&);
+  // cppcheck-suppress [duplInheritedMember]
   static constexpr ResultType Run() {
     return &call_with_indices<ReturnType, FunctionObject,
                               (BoundIndices - 1)...>;
@@ -722,6 +723,7 @@ struct VariantCoreAccess {
       Self* self, Args&&... args) {
     Destroy(*self);
     using New = typename absl::OTABSL_OPTION_NAMESPACE_NAME::variant_alternative<NewIndex, Self>::type;
+    // cppcheck-suppress [legacyUninitvar]
     New* const result = ::new (static_cast<void*>(&self->state_))
         New(absl::OTABSL_OPTION_NAMESPACE_NAME::forward<Args>(args)...);
     self->index_ = NewIndex;
@@ -1310,6 +1312,7 @@ class VariantStateBaseDestructorNontrivial : protected VariantStateBase<T...> {
     VariantStateBaseDestructorNontrivial* self;
   };
 
+  // cppcheck-suppress [duplInheritedMember]
   void destroy() { VisitIndices<sizeof...(T)>::Run(Destroyer{this}, index_); }
 
   ~VariantStateBaseDestructorNontrivial() { destroy(); }

api/include/opentelemetry/nostd/shared_ptr.h

@@ -37,7 +37,7 @@ class shared_ptr
 
   struct alignas(kAlignment) PlacementBuffer
   {
-    char data[kMaxSize];
+    char data[kMaxSize]{};
   };
 
   class shared_ptr_wrapper

api/include/opentelemetry/plugin/tracer.h

@@ -20,7 +20,7 @@ class DynamicLibraryHandle;
 class Span final : public trace::Span
 {
 public:
-  Span(std::shared_ptr<trace::Tracer> &&tracer, nostd::shared_ptr<trace::Span> span) noexcept
+  Span(std::shared_ptr<trace::Tracer> &&tracer, const nostd::shared_ptr<trace::Span> &span) noexcept
       : tracer_{std::move(tracer)}, span_{span}
   {}
 

api/include/opentelemetry/trace/context.h

@@ -34,7 +34,8 @@ inline bool IsRootSpan(const context::Context &context) noexcept
 }
 
 // Set Span into explicit context
-inline context::Context SetSpan(context::Context &context, nostd::shared_ptr<Span> span) noexcept
+inline context::Context SetSpan(context::Context &context,
+                                const nostd::shared_ptr<Span> &span) noexcept
 {
   return context.SetValue(kSpanKey, span);
 }

api/include/opentelemetry/trace/default_span.h

@@ -63,7 +63,7 @@ class DefaultSpan : public Span
 
   nostd::string_view ToString() const noexcept { return "DefaultSpan"; }
 
-  DefaultSpan(SpanContext span_context) noexcept : span_context_(span_context) {}
+  DefaultSpan(SpanContext span_context) noexcept : span_context_(std::move(span_context)) {}
 
   // movable and copiable
   DefaultSpan(DefaultSpan &&spn) noexcept : Span(), span_context_(spn.GetContext()) {}

api/include/opentelemetry/trace/provider.h

@@ -36,7 +36,7 @@ class OPENTELEMETRY_EXPORT Provider
   /**
    * Changes the singleton TracerProvider.
    */
-  static void SetTracerProvider(nostd::shared_ptr<TracerProvider> tp) noexcept
+  static void SetTracerProvider(const nostd::shared_ptr<TracerProvider> &tp) noexcept
   {
     std::lock_guard<common::SpinLockMutex> guard(GetLock());
     GetProvider() = tp;

api/include/opentelemetry/trace/span_context.h

@@ -5,6 +5,8 @@
 
 #include <stdint.h>
 
+#include <utility>
+
 #include "opentelemetry/nostd/shared_ptr.h"
 #include "opentelemetry/trace/span_id.h"
 #include "opentelemetry/trace/trace_flags.h"
@@ -46,7 +48,7 @@ class SpanContext final
         span_id_(span_id),
         trace_flags_(trace_flags),
         is_remote_(is_remote),
-        trace_state_(trace_state)
+        trace_state_(std::move(trace_state))
   {}
 
   SpanContext(const SpanContext &ctx) = default;
@@ -64,7 +66,7 @@ class SpanContext final
   const trace::SpanId &span_id() const noexcept { return span_id_; }
 
   // @returns the trace_state associated with this span_context
-  const nostd::shared_ptr<trace::TraceState> trace_state() const noexcept { return trace_state_; }
+  const nostd::shared_ptr<trace::TraceState> &trace_state() const noexcept { return trace_state_; }
 
   /*
    * @param that SpanContext for comparing.

examples/grpc/tracer_common.h

@@ -44,7 +44,7 @@ class GrpcClientCarrier : public opentelemetry::context::propagation::TextMapCar
     context_->AddMetadata(std::string(key), std::string(value));
   }
 
-  ClientContext *context_;
+  ClientContext *context_ = nullptr;
 };
 
 class GrpcServerCarrier : public opentelemetry::context::propagation::TextMapCarrier
@@ -69,7 +69,7 @@ class GrpcServerCarrier : public opentelemetry::context::propagation::TextMapCar
     // Not required for server
   }
 
-  ServerContext *context_;
+  ServerContext *context_ = nullptr;
 };
 
 void InitTracer()

examples/http/server.h

@@ -19,13 +19,13 @@ class HttpServer : public HTTP_SERVER_NS::HttpRequestCallback
   std::atomic<bool> is_running_{false};
 
 public:
-  HttpServer(std::string server_name = "test_server", uint16_t port = 8800) : port_(port)
+  HttpServer(const std::string &server_name = "test_server", uint16_t port = 8800) : port_(port)
   {
     server_.setServerName(server_name);
     server_.setKeepalive(false);
   }
 
-  void AddHandler(std::string path, HTTP_SERVER_NS::HttpRequestCallback *request_handler)
+  void AddHandler(const std::string &path, HTTP_SERVER_NS::HttpRequestCallback *request_handler)
   {
     server_.addHandler(path, *request_handler);
   }

exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_record_exporter.h

@@ -52,12 +52,12 @@ struct ElasticsearchExporterOptions
    * from elasticsearch
    * @param console_debug If true, print the status of the exporter methods in the console
    */
-  ElasticsearchExporterOptions(std::string host         = "localhost",
-                               int port                 = 9200,
-                               std::string index        = "logs",
-                               int response_timeout     = 30,
-                               bool console_debug       = false,
-                               HttpHeaders http_headers = {})
+  ElasticsearchExporterOptions(const std::string &host         = "localhost",
+                               int port                        = 9200,
+                               const std::string &index        = "logs",
+                               int response_timeout            = 30,
+                               bool console_debug              = false,
+                               const HttpHeaders &http_headers = {})
       : host_{host},
         port_{port},
         index_{index},

exporters/otlp/src/otlp_file_client.cc

@@ -973,7 +973,6 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender
   explicit OtlpFileSystemBackend(const OtlpFileClientFileSystemOptions &options)
       : options_(options), is_initialized_{false}
   {
-    file_ = std::make_shared<FileStats>();
     file_->is_shutdown.store(false);
     file_->rotate_index            = 0;
     file_->written_size            = 0;
@@ -1541,9 +1540,9 @@ class OPENTELEMETRY_LOCAL_SYMBOL OtlpFileSystemBackend : public OtlpFileAppender
     std::mutex background_thread_waiter_lock;
     std::condition_variable background_thread_waiter_cv;
   };
-  std::shared_ptr<FileStats> file_;
+  std::shared_ptr<FileStats> file_ = std::make_shared<FileStats>();
 
-  std::atomic<bool> is_initialized_;
+  std::atomic<bool> is_initialized_{false};
   std::time_t check_file_path_interval_{0};
 };
 

exporters/otlp/src/otlp_http_client.cc

@@ -753,13 +753,7 @@ sdk::common::ExportResult OtlpHttpClient::Export(
   auto session = createSession(message, std::move(result_callback));
   if (opentelemetry::nostd::holds_alternative<sdk::common::ExportResult>(session))
   {
-    sdk::common::ExportResult result =
-        opentelemetry::nostd::get<sdk::common::ExportResult>(session);
-    if (result_callback)
-    {
-      result_callback(result);
-    }
-    return result;
+    return opentelemetry::nostd::get<sdk::common::ExportResult>(session);
   }
 
   addSession(std::move(opentelemetry::nostd::get<HttpSessionData>(session)));
@@ -904,9 +898,11 @@ OtlpHttpClient::createSession(
       {
         std::cerr << error_message << '\n';
       }
-      OTEL_INTERNAL_LOG_ERROR(error_message.c_str());
+      OTEL_INTERNAL_LOG_ERROR(error_message);
 
-      return opentelemetry::sdk::common::ExportResult::kFailure;
+      const auto result = opentelemetry::sdk::common::ExportResult::kFailure;
+      result_callback(result);
+      return result;
     }
 
     if (!parse_url.path_.empty() && parse_url.path_[0] == '/')
@@ -938,7 +934,10 @@ OtlpHttpClient::createSession(
         OTEL_INTERNAL_LOG_DEBUG("[OTLP HTTP Client] Serialize body failed(Binary):"
                                 << message.InitializationErrorString());
       }
-      return opentelemetry::sdk::common::ExportResult::kFailure;
+
+      const auto result = opentelemetry::sdk::common::ExportResult::kFailure;
+      result_callback(result);
+      return result;
     }
     content_type = kHttpBinaryContentType;
   }
@@ -971,7 +970,9 @@ OtlpHttpClient::createSession(
     }
     OTEL_INTERNAL_LOG_ERROR(error_message);
 
-    return opentelemetry::sdk::common::ExportResult::kFailure;
+    const auto result = opentelemetry::sdk::common::ExportResult::kFailure;
+    result_callback(result);
+    return result;
   }
 
   auto session = http_client_->CreateSession(options_.url);