From ccef700cca40d04d10ca907f39157287fcdf6ce6 Mon Sep 17 00:00:00 2001 From: Brett Jia Date: Fri, 17 Jan 2025 23:44:08 -0500 Subject: [PATCH] remove some usage of static variables --- src/linux/cosmotop_collect.cpp | 51 ++++++++++--------- .../intel_gpu_top/intel_name_lookup_shim.c | 9 ++-- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/linux/cosmotop_collect.cpp b/src/linux/cosmotop_collect.cpp index 569989b..557fade 100644 --- a/src/linux/cosmotop_collect.cpp +++ b/src/linux/cosmotop_collect.cpp @@ -1654,41 +1654,44 @@ namespace Gpu { namespace Intel { // Attempts to initialize with Intel PMU, returning the device name if successful - static char *init_pmu() { + static string init_pmu() { char *gpu_path = find_intel_gpu_dir(); if (!gpu_path) { Logger::debug("Failed to find Intel GPU sysfs path, Intel GPUs will not be detected"); - return nullptr; + return ""; } char *gpu_device_id = get_intel_device_id(gpu_path); + free(gpu_path); if (!gpu_device_id) { Logger::debug("Failed to find Intel GPU device ID, Intel GPUs will not be detected"); - return nullptr; + return ""; } - char *gpu_device_name = get_intel_device_name(gpu_device_id); - if (!gpu_device_name) { + char *dev_name = get_intel_device_name(gpu_device_id); + free(gpu_device_id); + + string gpu_device_name; + if (dev_name) { + gpu_device_name = string(dev_name); + free(dev_name); + } else { Logger::warning("Failed to find Intel GPU device name in internal database"); - gpu_device_name = strdup("Intel GPU"); + gpu_device_name = "Intel GPU"s; } - free(gpu_device_id); - engines = discover_engines(device); if (!engines) { Logger::debug("Failed to find Intel GPU engines, Intel GPUs will not be detected"); - free(gpu_device_name); - return nullptr; + return ""; } int ret = pmu_init(engines); if (ret) { Logger::warning("Intel GPU: Failed to initialize PMU"); - free(gpu_device_name); free_engines(engines); engines = nullptr; - return nullptr; + return ""; } return gpu_device_name; @@ -1720,11 +1723,11 @@ namespace Gpu { } // Attempts to initialize with intel_gpu_exporter REST api - static char *init_rest() { + static string init_rest() { const auto rest_endpoint = Config::getS("intel_gpu_exporter"); if (rest_endpoint.empty()) { Logger::debug("Fallback Intel GPU exporter not configured, Intel GPUs will not be detected"); - return nullptr; + return ""; } try { @@ -1737,7 +1740,7 @@ namespace Gpu { Logger::warning("Failed to get Intel GPU device ID from exporter"); delete exporter; exporter = nullptr; - return nullptr; + return ""; } // We get the value as a float, so need to convert it to hex string @@ -1748,25 +1751,28 @@ namespace Gpu { char *gpu_device_name = get_intel_device_name(ss.str().c_str()); if (!gpu_device_name) { Logger::warning("Failed to find Intel GPU device name in internal database"); - gpu_device_name = strdup("Intel GPU"); + return "Intel GPU"s; } - return gpu_device_name; + string result = string(gpu_device_name); + free(gpu_device_name); + + return result; } catch (const std::exception &e) { Logger::warning("Failed to connect to Intel GPU exporter: "s + e.what()); if (exporter) delete exporter; exporter = nullptr; - return nullptr; + return ""; } } bool init() { if (initialized) return false; - char *gpu_device_name = init_pmu(); - if (!gpu_device_name) { + string gpu_device_name = init_pmu(); + if (gpu_device_name.empty()) { gpu_device_name = init_rest(); - if (!gpu_device_name) return false; + if (gpu_device_name.empty()) return false; } if (engines) { @@ -1778,8 +1784,7 @@ namespace Gpu { gpus.resize(gpus.size() + device_count); gpu_names.resize(gpus.size() + device_count); - gpu_names[Nvml::device_count + Rsmi::device_count] = string(gpu_device_name); - free(gpu_device_name); + gpu_names[Nvml::device_count + Rsmi::device_count] = gpu_device_name; initialized = true; Intel::collect<1>(gpus.data() + Nvml::device_count + Rsmi::device_count); diff --git a/src/linux/intel_gpu_top/intel_name_lookup_shim.c b/src/linux/intel_gpu_top/intel_name_lookup_shim.c index c4b831d..f1fb089 100644 --- a/src/linux/intel_gpu_top/intel_name_lookup_shim.c +++ b/src/linux/intel_gpu_top/intel_name_lookup_shim.c @@ -28,10 +28,11 @@ #define VENDOR_FILE "vendor" #define DEVICE_FILE "device" +// Caller must free the returned pointer char* find_intel_gpu_dir() { DIR *dir; struct dirent *entry; - static char path[256]; + char path[256]; char vendor_path[256]; char vendor_id[16]; @@ -57,7 +58,7 @@ char* find_intel_gpu_dir() { closedir(dir); // Return the parent directory (i.e., /sys/class/drm/card*) snprintf(path, sizeof(path), "%s/%s", SYSFS_PATH, entry->d_name); - return path; + return strdup(path); } } fclose(file); @@ -69,8 +70,9 @@ char* find_intel_gpu_dir() { return NULL; // Intel GPU not found } +// Caller must free the returned pointer char* get_intel_device_id(const char* gpu_dir) { - static char device_path[256]; + char device_path[256]; char device_id[16]; // Construct the path to the device file @@ -93,6 +95,7 @@ char* get_intel_device_id(const char* gpu_dir) { return NULL; } +// Caller must free the returned pointer char *get_intel_device_name(const char *device_id) { uint16_t devid = strtol(device_id, NULL, 16); char dev_name[256];