Removal of C idioms

bench/collatz/collatz.cc

@@ -186,7 +186,7 @@ int main(int c, char **v) {
   int fd_statsd = -1;
   char *path_statsd = NULL;
   if ((path_statsd = getenv("DD_DOGSTATSD_SOCKET"))) {
-    statsd_connect(path_statsd, strlen(path_statsd), &fd_statsd);
+    statsd_connect(std::string_view(path_statsd), &fd_statsd);
   }
 
   // OK, so we want to wait until everyone has started, but if we have more

include/ddprof_context.hpp

@@ -15,9 +15,6 @@
 #include <sched.h>
 #include <unistd.h>
 
-// forward declarations
-typedef struct StackHandler StackHandler;
-
 struct DDProfContext_V2 {
   DDProfContext_V2() = default;
   ~DDProfContext_V2() {
@@ -55,32 +52,3 @@ struct DDProfContext_V2 {
   DDProfContext_V2(DDProfContext_V2 &&other) = delete;
   DDProfContext_V2 &operator=(const DDProfContext_V2 &&) = delete;
 };
-
-struct DDProfContext {
-  struct {
-    bool enable;
-    double upload_period;
-    bool fault_info;
-    bool core_dumps;
-    int nice;
-    int num_cpu;
-    pid_t pid; // ! only use for perf attach (can be -1 in global mode)
-    bool global;
-    uint32_t worker_period; // exports between worker refreshes
-    int dd_profiling_fd;    // opened file descriptor to our internal lib
-    int sockfd;
-    bool wait_on_socket;
-    bool show_samples;
-    cpu_set_t cpu_affinity;
-    const char *switch_user;
-    const char *internal_stats;
-    const char *tags;
-  } params;
-
-  bool initialized;
-  ExporterInput exp_input;
-  PerfWatcher watchers[MAX_TYPE_WATCHER];
-  int num_watchers;
-  void *callback_ctx; // user state to be used in callback (lib mode)
-  DDProfWorkerContext worker_ctx;
-};

include/ddprof_stats.hpp

@@ -64,7 +64,7 @@ DDRes ddprof_stats_clear(unsigned int stat);
 DDRes ddprof_stats_get(unsigned int stat, long *out);
 
 // Send all the registered values
-DDRes ddprof_stats_send(const char *statsd_socket);
+DDRes ddprof_stats_send(std::string_view statsd_socket);
 
 // Print all known stats to the configured log
-void ddprof_stats_print();
+void ddprof_stats_print();

include/ddprof_worker_context.hpp

@@ -22,20 +22,20 @@ typedef struct UserTags UserTags;
 // Mutable states within a worker
 struct DDProfWorkerContext {
   // Persistent reference to the state shared accross workers
-  PersistentWorkerState *persistent_worker_state;
-  PEventHdr pevent_hdr;   // perf_event buffer holder
-  DDProfExporter *exp[2]; // wrapper around rust exporter
-  DDProfPProf *pprof[2];  // wrapper around rust exporter
-  int i_current_pprof;
-  volatile bool exp_error;
-  pthread_t exp_tid;
-  UnwindState *us;
-  UserTags *user_tags;
-  ProcStatus proc_status;
+  PersistentWorkerState *persistent_worker_state{nullptr};
+  PEventHdr pevent_hdr;     // perf_event buffer holder
+  DDProfExporter *exp[2]{}; // wrapper around rust exporter
+  DDProfPProf *pprof[2]{};  // wrapper around rust exporter
+  int i_current_pprof{0};
+  volatile bool exp_error{false};
+  pthread_t exp_tid{0};
+  UnwindState *us{};
+  UserTags *user_tags{};
+  ProcStatus proc_status{};
   std::chrono::steady_clock::time_point
-      cycle_start_time;  // time at which current export cycle was started
-  int64_t send_nanos;    // Last time an export was sent
-  uint32_t count_worker; // exports since last cache clear
-  std::array<uint64_t, MAX_TYPE_WATCHER> lost_events_per_watcher;
+      cycle_start_time{};   // time at which current export cycle was started
+  int64_t send_nanos{0};    // Last time an export was sent
+  uint32_t count_worker{0}; // exports since last cache clear
+  std::array<uint64_t, MAX_TYPE_WATCHER> lost_events_per_watcher{};
   ddprof::LiveAllocation live_allocation;
 };

include/exporter/ddprof_exporter.hpp

@@ -28,7 +28,7 @@ typedef struct DDProfExporter {
   int32_t _nb_consecutive_errors{0};
 } DDProfExporter;
 
-DDRes ddprof_exporter_init(const ExporterInput *exporter_input,
+DDRes ddprof_exporter_init(const ExporterInput &exporter_input,
                            DDProfExporter *exporter);
 
 DDRes ddprof_exporter_new(const UserTags *user_tags, DDProfExporter *exporter);

include/exporter_input.hpp

@@ -24,7 +24,6 @@ struct ExporterInput {
   std::string port; // port appended to the host IP (ignored in agentless)
   std::string debug_pprof_prefix; // local pprof prefix (debug)
   bool do_export;                 // prevent exports if needed (debug flag)
-
   std::string_view user_agent{
       USERAGENT_DEFAULT}; // ignored for now (override in shared lib)
   std::string_view language{
@@ -33,3 +32,22 @@ struct ExporterInput {
   std::string_view profiler_version;
   bool agentless; // Whether or not to actually use API key/intake
 };
+
+#ifdef DEBUG
+void print(const ExporterInput &input) {
+  std::cout << "API Key: " << input.api_key << "\n";
+  std::cout << "Environment: " << input.environment << "\n";
+  std::cout << "Service: " << input.service << "\n";
+  std::cout << "Service Version: " << input.service_version << "\n";
+  std::cout << "Host: " << input.host << "\n";
+  std::cout << "URL: " << input.url << "\n";
+  std::cout << "Port: " << input.port << "\n";
+  std::cout << "Debug pprof prefix: " << input.debug_pprof_prefix << "\n";
+  std::cout << "Do Export: " << (input.do_export ? "true" : "false") << "\n";
+  std::cout << "User Agent: " << input.user_agent << "\n";
+  std::cout << "Language: " << input.language << "\n";
+  std::cout << "Family: " << input.family << "\n";
+  std::cout << "Profiler Version: " << input.profiler_version << "\n";
+  std::cout << "Agentless: " << (input.agentless ? "true" : "false") << "\n";
+}
+#endif

include/presets.hpp

@@ -5,9 +5,11 @@
 
 #pragma once
 
-#include "ddprof_context.hpp"
 #include "ddres_def.hpp"
+#include "perf_watcher.hpp"
 #include <cstddef>
+#include <string_view>
+#include <vector>
 
 namespace ddprof {
 
@@ -16,8 +18,6 @@ struct Preset {
   const char *events;
 };
 
-DDRes add_preset(DDProfContext *ctx, const char *preset,
-                 bool pid_or_global_mode);
 DDRes add_preset_v2(std::string_view preset, bool pid_or_global_mode,
                     std::vector<PerfWatcher> &watchers);
 

include/statsd.hpp

@@ -6,6 +6,7 @@
 #pragma once
 
 #include <stddef.h>
+#include <string_view>
 
 #include "ddres_def.hpp"
 
@@ -18,7 +19,7 @@ typedef enum STAT_TYPES {
 
 /// Connect to a statsd server, returning a ddres and populating the passed
 /// pointer on success
-DDRes statsd_connect(const char *, size_t, int *);
+DDRes statsd_connect(std::string_view statsd_socket, int *);
 
 /// Send the stats in a statsd format, returns a ddres
 DDRes statsd_send(int, const char *, void *, int);
@@ -27,4 +28,4 @@ DDRes statsd_send(int, const char *, void *, int);
 DDRes statsd_close(int);
 
 /* Private */
-DDRes statsd_listen(const char *path, size_t sz_path, int *fd);
+DDRes statsd_listen(std::string_view path, int *fd);

src/ddprof_context_lib.cc

@@ -45,7 +45,7 @@ static void order_watchers(ddprof::span<PerfWatcher> watchers) {
 namespace {
 void copy_cli_values(const DDProfCLI &ddprof_cli, DDProfContext_V2 &ctx) {
 
-  // possible std::move here ?
+  // Do we want to std::move more ?
   ctx.exp_input = ddprof_cli.exporter_input;
   // todo avoid manual copies
   ctx.params.tags = ddprof_cli.tags;
@@ -65,10 +65,6 @@ void copy_cli_values(const DDProfCLI &ddprof_cli, DDProfContext_V2 &ctx) {
   // Debug
   ctx.params.internal_stats = ddprof_cli.internal_stats;
   ctx.params.enable = ddprof_cli.enable;
-  // todo check if this is reasonable ?
-  if (!ddprof_cli.enable) {
-    setenv("DD_PROFILING_ENABLED", "false", true);
-  }
   // hidden
   if (!ddprof_cli.cpu_affinity.empty() &&
       !parse_cpu_mask(ddprof_cli.cpu_affinity, ctx.params.cpu_affinity)) {

src/ddprof_stats.cc

@@ -112,15 +112,14 @@ DDRes ddprof_stats_get(unsigned int stat, long *out) {
   return ddres_init();
 }
 
-DDRes ddprof_stats_send(const char *statsd_socket) {
-  if (!statsd_socket) {
+DDRes ddprof_stats_send(std::string_view statsd_socket) {
+  if (statsd_socket.empty()) {
     LG_NTC("No statsd socket provided");
     return ddres_init();
   }
   int fd_statsd = -1;
 
-  if (IsDDResNotOK(
-          statsd_connect(statsd_socket, strlen(statsd_socket), &fd_statsd))) {
+  if (IsDDResNotOK(statsd_connect(statsd_socket, &fd_statsd))) {
     // Invalid socket. No use trying to send data (and avoid flood of logs).
     return ddres_init();
   }

src/ddprof_worker.cc

@@ -499,7 +499,7 @@ DDRes ddprof_worker_cycle(DDProfContext_V2 &ctx, int64_t now,
 
   // And emit diagnostic output (if it's enabled)
   print_diagnostics(ctx.worker_ctx.us->dso_hdr);
-  if (IsDDResNotOK(ddprof_stats_send(ctx.params.internal_stats.c_str()))) {
+  if (IsDDResNotOK(ddprof_stats_send(ctx.params.internal_stats))) {
     LG_WRN("Unable to utilize to statsd socket.  Suppressing future stats.");
     ctx.params.internal_stats = {};
   }
@@ -614,8 +614,8 @@ DDRes ddprof_worker_init(DDProfContext_V2 &ctx,
                          PersistentWorkerState *persistent_worker_state) {
   try {
     DDRES_CHECK_FWD(worker_library_init(ctx, persistent_worker_state));
-    ctx.worker_ctx.exp[0] = (DDProfExporter *)calloc(1, sizeof(DDProfExporter));
-    ctx.worker_ctx.exp[1] = (DDProfExporter *)calloc(1, sizeof(DDProfExporter));
+    ctx.worker_ctx.exp[0] = new DDProfExporter();
+    ctx.worker_ctx.exp[1] = new DDProfExporter();
     ctx.worker_ctx.pprof[0] = new DDProfPProf();
     ctx.worker_ctx.pprof[1] = new DDProfPProf();
     if (!ctx.worker_ctx.exp[0] || !ctx.worker_ctx.exp[1]) {
@@ -626,10 +626,8 @@ DDRes ddprof_worker_init(DDProfContext_V2 &ctx,
       DDRES_RETURN_ERROR_LOG(DD_WHAT_BADALLOC, "Error creating exporter");
     }
 
-    DDRES_CHECK_FWD(
-        ddprof_exporter_init(&ctx.exp_input, ctx.worker_ctx.exp[0]));
-    DDRES_CHECK_FWD(
-        ddprof_exporter_init(&ctx.exp_input, ctx.worker_ctx.exp[1]));
+    DDRES_CHECK_FWD(ddprof_exporter_init(ctx.exp_input, ctx.worker_ctx.exp[0]));
+    DDRES_CHECK_FWD(ddprof_exporter_init(ctx.exp_input, ctx.worker_ctx.exp[1]));
     // warning : depends on unwind init
     DDRES_CHECK_FWD(
         ddprof_exporter_new(ctx.worker_ctx.user_tags, ctx.worker_ctx.exp[0]));
@@ -662,7 +660,7 @@ DDRes ddprof_worker_free(DDProfContext_V2 &ctx) {
     for (int i = 0; i < 2; i++) {
       if (ctx.worker_ctx.exp[i]) {
         DDRES_CHECK_FWD(ddprof_exporter_free(ctx.worker_ctx.exp[i]));
-        free(ctx.worker_ctx.exp[i]);
+        delete ctx.worker_ctx.exp[i];
         ctx.worker_ctx.exp[i] = nullptr;
       }
       if (ctx.worker_ctx.pprof[i]) {

src/exe/main.cc

@@ -441,7 +441,7 @@ int main(int argc, char *argv[]) {
   // Execute manages its own return path
   std::vector<char *> cmd_line = cli.get_user_command_line();
   defer { ddprof::free_user_command_line(cmd_line); };
-  if (-1 == execvp(*argv, (char *const *)argv)) {
+  if (-1 == execvp(cmd_line[0], cmd_line.data())) {
     // Logger is not configured in the context of the parent process:
     // We use stderr as a standard logging mechanism
     switch (errno) {

src/exporter/ddprof_exporter.cc

@@ -26,12 +26,13 @@
 
 static const int k_timeout_ms = 10000;
 
-std::string alloc_url_agent(const char *protocol, const char *host,
-                            const std::string &port) {
+std::string alloc_url_agent(std::string_view protocol, std::string_view host,
+                            std::string_view port) {
   if (!port.empty()) {
-    return ddprof::string_format("%s%s:%s", protocol, host, port.c_str());
+    return ddprof::string_format("%s%s:%s", protocol.data(), host.data(),
+                                 port.data());
   } else {
-    return ddprof::string_format("%s%s", protocol, host);
+    return ddprof::string_format("%s%s", protocol.data(), host.data());
   }
 }
 
@@ -72,8 +73,11 @@ static DDRes write_pprof_file(const ddog_prof_EncodedProfile *encoded_profile,
   return {};
 }
 
-bool contains_port(const char *url) {
-  const char *port_ptr = strrchr(url, ':');
+bool contains_port(std::string_view url) {
+  if (url.empty()) {
+    return false;
+  }
+  const char *port_ptr = strrchr(url.data(), ':');
   if (port_ptr != NULL) {
     // Check if the characters after the ':' are digits
     for (const char *p = port_ptr + 1; *p != '\0'; p++) {
@@ -86,15 +90,17 @@ bool contains_port(const char *url) {
     return false;
   }
 }
+#include <iostream>
 
-DDRes ddprof_exporter_init(const ExporterInput *exporter_input,
+
+DDRes ddprof_exporter_init(const ExporterInput &exporter_input,
                            DDProfExporter *exporter) {
-  exporter->_input = *exporter_input;
+  exporter->_input = exporter_input;
   // if we have an API key we assume we are heading for intake (slightly
   // fragile #TODO add a parameter)
 
-  if (exporter_input->agentless && !exporter_input->api_key.empty() &&
-      exporter_input->api_key.size() >= k_size_api_key) {
+  if (exporter_input.agentless && !exporter_input.api_key.empty() &&
+      exporter_input.api_key.size() >= k_size_api_key) {
     LG_NTC("[EXPORTER] Targeting intake instead of agent (API Key available)");
     exporter->_agent = false;
   } else {
@@ -103,30 +109,29 @@ DDRes ddprof_exporter_init(const ExporterInput *exporter_input,
   }
 
   if (exporter->_agent) {
-    std::string port_str = exporter_input->port;
+    std::string port_str = exporter_input.port;
 
-    if (!exporter_input->url.empty()) {
+    if (!exporter_input.url.empty()) {
       // uds -> no port
-      if (!strncasecmp(exporter_input->url.c_str(), "unix", 4)) {
+      if (!strncasecmp(exporter_input.url.c_str(), "unix", 4)) {
         port_str = {};
       }
       // already port -> no port
-      else if (contains_port(exporter_input->url.c_str())) {
+      else if (contains_port(exporter_input.url)) {
         port_str = {};
       }
       // check if schema is already available
-      if (strstr(exporter_input->url.c_str(), "://") != NULL) {
-        exporter->_url =
-            alloc_url_agent("", exporter_input->url.c_str(), port_str);
+      if (strstr(exporter_input.url.c_str(), "://") != NULL) {
+        exporter->_url = alloc_url_agent("", exporter_input.url, port_str);
       } else {
         // not available, assume http
         exporter->_url =
-            alloc_url_agent("http://", exporter_input->url.c_str(), port_str);
+            alloc_url_agent("http://", exporter_input.url, port_str);
       }
     } else {
       // no url, use default host and port settings
-      exporter->_url = alloc_url_agent("http://", exporter_input->host.c_str(),
-                                       exporter_input->port);
+      exporter->_url =
+          alloc_url_agent("http://", exporter_input.host, exporter_input.port);
     }
   } else {
     // agentless mode
@@ -138,9 +143,9 @@ DDRes ddprof_exporter_init(const ExporterInput *exporter_input,
       LG_WRN(
           "[EXPORTER] Agentless - Attempting to use host (%s) instead of empty "
           "url",
-          exporter_input->host.c_str());
-      exporter->_url = alloc_url_agent("http://", exporter_input->host.c_str(),
-                                       exporter_input->port);
+          exporter_input.host.c_str());
+      exporter->_url = alloc_url_agent("http://", exporter_input.host.c_str(),
+                                       exporter_input.port);
     }
   }
   if (exporter->_url.empty()) {

src/lib/dd_profiling.cc

@@ -141,7 +141,7 @@ int exec_ddprof(pid_t target_pid, pid_t parent_pid, int sock_fd) {
   snprintf(sock_buf, sizeof(sock_buf), "%d", sock_fd);
 
   char pid_opt_str[] = "-p";
-  char sock_opt_str[] = "-z";
+  char sock_opt_str[] = "--socket";
 
   // cppcheck-suppress variableScope
   char *argv[] = {ddprof_str,   pid_opt_str, pid_buf,