@cryptotaxi247 / netdata-1 / commits / 959c15f9b

Code improvement (Windows.plugin) (#21523)

thiagoftsm committed Jan 12, 2026 at 16:06 UTC 959c15f9b3fd50f5cfee51c491770f6aa6b41c76
2 files changed +111 -97
src/collectors/windows.plugin/GetHardwareInfo.c
+105 -94
@@ -6,7 +6,7 @@
6 #include "netdata_win_driver.h"
7
8 static const char *srv_name = "NetdataDriver";
9 -const char *drv_path = "%SystemRoot%\\system32\\netdata_driver.sys";
9 +static const char *drv_path = "%SystemRoot%\\system32\\netdata_driver.sys";
10
11 struct cpu_data {
12 RRDDIM *rd_cpu_temp;
@@ -15,45 +15,39 @@ struct cpu_data {
15 };
16
17 struct cpu_data *cpus = NULL;
18 -size_t ncpus = 0 ;
18 +size_t ncpus = 0;
19 static ND_THREAD *hardware_info_thread = NULL;
20 static collected_number (*temperature_fcnt)(MSR_REQUEST *) = NULL;
21 +static CRITICAL_SECTION cpus_lock;
22 +bool cpus_lock_initialized = false;
23
24 static void netdata_stop_driver()
25 {
26 SC_HANDLE scm = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
27 if (scm == NULL) {
26 - nd_log(
27 - NDLS_COLLECTORS,
28 - NDLP_ERR,
29 - "Cannot open Service Manager. Error= %lu \n", GetLastError());
28 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot open Service Manager. Error= %lu \n", GetLastError());
29 return;
30 }
31
32 SC_HANDLE service = OpenService(scm, srv_name, SERVICE_STOP | SERVICE_QUERY_STATUS | DELETE);
33 if (service == NULL) {
35 - nd_log(
36 - NDLS_COLLECTORS,
37 - NDLP_ERR,
38 - "Cannot open the service. Error= %lu \n", GetLastError());
34 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot open the service. Error= %lu \n", GetLastError());
35 CloseServiceHandle(scm);
36 return;
37 }
38
43 - SERVICE_STATUS_PROCESS ss_status = {};
44 - if (ControlService(service, SERVICE_CONTROL_STOP, (LPSERVICE_STATUS)&ss_status) == 0) {
45 - if (GetLastError() != ERROR_SERVICE_NOT_ACTIVE) {
46 - nd_log(
47 - NDLS_COLLECTORS,
48 - NDLP_ERR,
49 - "Cannot stop the service. Error= %lu \n", GetLastError());
39 + SERVICE_STATUS ss_status = {};
40 + if (ControlService(service, SERVICE_CONTROL_STOP, &ss_status) == 0) {
41 + DWORD err = GetLastError();
42 + if (err != ERROR_SERVICE_NOT_ACTIVE) {
43 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot stop the service. Error= %lu \n", err);
44 }
51 - } else {
52 - if (!DeleteService(service)) {
53 - nd_log(
54 - NDLS_COLLECTORS,
55 - NDLP_ERR,
56 - "Cannot delete service. Error= %lu \n", GetLastError());
45 + }
46 +
47 + if (!DeleteService(service)) {
48 + DWORD err = GetLastError();
49 + if (err != ERROR_SERVICE_MARKED_FOR_DELETE) {
50 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot delete service. Error= %lu \n", err);
51 }
52 }
53
@@ -63,46 +57,35 @@ static void netdata_stop_driver()
57
58 int netdata_install_driver()
59 {
66 - SC_HANDLE scm = OpenSCManager(
67 - NULL,
68 - NULL,
69 - SC_MANAGER_CREATE_SERVICE
70 - );
60 + SC_HANDLE scm = OpenSCManager(NULL, NULL, SC_MANAGER_CREATE_SERVICE);
61
62 if (unlikely(!scm)) {
73 - nd_log(
74 - NDLS_COLLECTORS,
75 - NDLP_ERR,
76 - "Cannot open Service Manager. Error= %lu \n", GetLastError());
63 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot open Service Manager. Error= %lu \n", GetLastError());
64 return -1;
65 }
66
67 char expanded_path[MAX_PATH];
68 if (ExpandEnvironmentStringsA(drv_path, expanded_path, sizeof(expanded_path)) == 0) {
82 - nd_log(
83 - NDLS_COLLECTORS,
84 - NDLP_ERR,
85 - "Cannot expand environment strings. Error= %lu \n", GetLastError());
69 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot expand environment strings. Error= %lu \n", GetLastError());
70 CloseServiceHandle(scm);
71 return -1;
72 }
73
74 // Create the service entry for the driver
75 SC_HANDLE service = CreateServiceA(
92 - scm,
93 - srv_name,
94 - srv_name,
95 - SERVICE_START | SERVICE_STOP | DELETE,
96 - SERVICE_KERNEL_DRIVER,
97 - SERVICE_DEMAND_START,
98 - SERVICE_ERROR_NORMAL,
99 - expanded_path,
100 - NULL,
101 - NULL,
102 - NULL,
103 - NULL,
104 - NULL
105 - );
76 + scm,
77 + srv_name,
78 + srv_name,
79 + SERVICE_START | SERVICE_STOP | DELETE,
80 + SERVICE_KERNEL_DRIVER,
81 + SERVICE_DEMAND_START,
82 + SERVICE_ERROR_NORMAL,
83 + expanded_path,
84 + NULL,
85 + NULL,
86 + NULL,
87 + NULL,
88 + NULL);
89
90 if (unlikely(!service)) {
91 if (GetLastError() == ERROR_SERVICE_EXISTS) {
@@ -110,10 +93,7 @@ int netdata_install_driver()
93 return 0;
94 }
95
113 - nd_log(
114 - NDLS_COLLECTORS,
115 - NDLP_ERR,
116 - "Cannot create Service. Error= %lu \n", GetLastError());
96 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot create Service. Error= %lu \n", GetLastError());
97 CloseServiceHandle(scm);
98 return -1;
99 }
@@ -128,31 +108,22 @@ int netdata_start_driver()
108 {
109 SC_HANDLE scm = OpenSCManagerA(NULL, NULL, SC_MANAGER_CONNECT);
110 if (unlikely(!scm)) {
131 - nd_log(
132 - NDLS_COLLECTORS,
133 - NDLP_ERR,
134 - "Cannot open Service Manager. Error= %lu \n", GetLastError());
111 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot open Service Manager. Error= %lu \n", GetLastError());
112 return -1;
113 }
114
115 SC_HANDLE service = OpenServiceA(scm, srv_name, SERVICE_START | SERVICE_QUERY_STATUS);
116 if (unlikely(!service)) {
117 CloseServiceHandle(scm);
141 - nd_log(
142 - NDLS_COLLECTORS,
143 - NDLP_ERR,
144 - "Cannot open Service. Error= %lu \n", GetLastError());
118 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot open Service. Error= %lu \n", GetLastError());
119 return -1;
120 }
121
122 int ret = 0;
123 if (!StartServiceA(service, 0, NULL)) {
124 DWORD err = GetLastError();
151 - if (err != ERROR_SERVICE_EXISTS && err != ERROR_SERVICE_ALREADY_RUNNING) {
152 - nd_log(
153 - NDLS_COLLECTORS,
154 - NDLP_ERR,
155 - "Cannot start Service. Error= %lu \n", err);
125 + if (err != ERROR_SERVICE_ALREADY_RUNNING) {
126 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot start Service. Error= %lu \n", err);
127 ret = -1;
128 }
129 }
@@ -164,28 +135,42 @@ int netdata_start_driver()
135
136 static inline HANDLE netdata_open_device()
137 {
167 - HANDLE msr_h = CreateFileA(MSR_USER_PATH, GENERIC_READ | GENERIC_WRITE, 0,
168 - NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
138 + HANDLE msr_h =
139 + CreateFileA(MSR_USER_PATH, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
140 if (msr_h == INVALID_HANDLE_VALUE) {
170 - nd_log(
171 - NDLS_COLLECTORS,
172 - NDLP_ERR,
173 - "Cannot open device. GetLastError= %lu \n", GetLastError());
141 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "Cannot open device. GetLastError= %lu \n", GetLastError());
142 }
143 return msr_h;
144 }
145
146 static collected_number netdata_intel_cpu_temp(MSR_REQUEST *req)
147 {
148 + if (!req)
149 + return 0;
150 +
151 const ULONG TJMAX = 100;
181 - ULONG digital_readout = (req->low >> 16) & 0x7F; // bits [22:16]
182 - return (collected_number)(TJMAX - digital_readout);
152 + ULONG digital_readout = (req->low >> 16) & 0x7F; // bits [22:16]
153 +
154 + collected_number temp = (collected_number)(TJMAX - digital_readout);
155 +
156 + if (temp < 0 || temp > 150)
157 + return 0;
158 +
159 + return temp;
160 }
161
162 static collected_number netdata_amd_cpu_temp(MSR_REQUEST *req)
163 {
164 + if (!req)
165 + return 0;
166 +
167 ULONG amd_temp = (req->low >> 21) & 0x7FF;
188 - return (collected_number) amd_temp/8;
168 + collected_number temp = (collected_number)amd_temp / 8;
169 +
170 + if (temp < 0 || temp > 150)
171 + return 0;
172 +
173 + return temp;
174 }
175
176 void netdata_collect_cpu_chart()
@@ -197,14 +182,21 @@ void netdata_collect_cpu_chart()
182
183 const uint32_t MSR_THERM_STATUS = 0x19C;
184
185 + EnterCriticalSection(&cpus_lock);
186 for (size_t cpu = 0; cpu < ncpus; cpu++) {
187 DWORD bytes = 0;
202 - MSR_REQUEST req = { MSR_THERM_STATUS, (ULONG)cpu, 0, 0 };
188 + MSR_REQUEST req = {MSR_THERM_STATUS, (ULONG)cpu, 0, 0};
189
190 if (DeviceIoControl(device, IOCTL_MSR_READ, &req, sizeof(req), &req, sizeof(req), &bytes, NULL)) {
205 - cpus[cpu].cpu_temp = temperature_fcnt(&req);
191 + if (temperature_fcnt)
192 + cpus[cpu].cpu_temp = temperature_fcnt(&req);
193 + else
194 + cpus[cpu].cpu_temp = 0;
195 + } else {
196 + cpus[cpu].cpu_temp = 0;
197 }
198 }
199 + LeaveCriticalSection(&cpus_lock);
200
201 CloseHandle(device);
202 }
@@ -248,6 +240,9 @@ static void netdata_detect_cpu()
240
241 static int initialize()
242 {
243 + InitializeCriticalSection(&cpus_lock);
244 + cpus_lock_initialized = true;
245 +
246 netdata_detect_cpu();
247 if (!temperature_fcnt) {
248 return -1;
@@ -264,7 +259,8 @@ static int initialize()
259 ncpus = os_get_system_cpus();
260 cpus = callocz(ncpus, sizeof(struct cpu_data));
261
267 - hardware_info_thread = nd_thread_create("hi_threads", NETDATA_THREAD_OPTION_DEFAULT, get_hardware_info_thread, NULL);
262 + hardware_info_thread =
263 + nd_thread_create("hw_info_thread", NETDATA_THREAD_OPTION_DEFAULT, get_hardware_info_thread, NULL);
264
265 return 0;
266 }
@@ -274,18 +270,18 @@ static RRDSET *netdata_publish_cpu_chart(int update_every)
270 static RRDSET *st_cpu_temp = NULL;
271 if (!st_cpu_temp) {
272 st_cpu_temp = rrdset_create_localhost(
277 - "cpu",
278 - "temperature",
279 - NULL,
280 - "temperature",
281 - "cpu.temperature",
282 - "Core temperature",
283 - "Celsius",
284 - PLUGIN_WINDOWS_NAME,
285 - "GetHardwareInfo",
286 - NETDATA_CHART_PRIO_CPU_TEMPERATURE,
287 - update_every,
288 - RRDSET_TYPE_LINE);
273 + "cpu",
274 + "temperature",
275 + NULL,
276 + "temperature",
277 + "cpu.temperature",
278 + "Core temperature",
279 + "Celsius",
280 + PLUGIN_WINDOWS_NAME,
281 + "GetHardwareInfo",
282 + NETDATA_CHART_PRIO_CPU_TEMPERATURE,
283 + update_every,
284 + RRDSET_TYPE_LINE);
285 }
286
287 return st_cpu_temp;
@@ -294,6 +290,8 @@ static RRDSET *netdata_publish_cpu_chart(int update_every)
290 static void netdata_loop_cpu_chart(int update_every)
291 {
292 RRDSET *chart = netdata_publish_cpu_chart(update_every);
293 +
294 + EnterCriticalSection(&cpus_lock);
295 for (int i = 0; i < (int)ncpus; i++) {
296 struct cpu_data *lcpu = &cpus[i];
297 if (!lcpu->rd_cpu_temp) {
@@ -303,7 +301,9 @@ static void netdata_loop_cpu_chart(int update_every)
301 }
302 rrddim_set_by_pointer(chart, lcpu->rd_cpu_temp, lcpu->cpu_temp);
303 }
306 - rrdset_done(chart);
304 + LeaveCriticalSection(&cpus_lock);
305 +
306 + rrdset_done(chart);
307 }
308
309 int do_GetHardwareInfo(int update_every, usec_t dt __maybe_unused)
@@ -323,8 +323,19 @@ int do_GetHardwareInfo(int update_every, usec_t dt __maybe_unused)
323
324 void do_GetHardwareInfo_cleanup()
325 {
326 - if (nd_thread_join(hardware_info_thread))
327 - nd_log_daemon(NDLP_ERR, "Failed to join Get Hardware Info thread");
326 + if (hardware_info_thread) {
327 + if (nd_thread_join(hardware_info_thread))
328 + nd_log_daemon(NDLP_ERR, "Failed to join Get Hardware Info thread");
329 + }
330
331 netdata_stop_driver();
332 +
333 + if (cpus_lock_initialized)
334 + DeleteCriticalSection(&cpus_lock);
335 +
336 + if (cpus) {
337 + freez(cpus);
338 + cpus = NULL;
339 + ncpus = 0;
340 + }
341 }
src/collectors/windows.plugin/GetPowerSupply.c
+6 -3
@@ -40,7 +40,7 @@ static inline void netdata_update_power_supply_values(
40 if (bs.Capacity != BATTERY_UNKNOWN_CAPACITY) {
41 NETDATA_DOUBLE num = bs.Capacity;
42 NETDATA_DOUBLE den = bi->FullChargedCapacity;
43 - num = (den) ? num/ den : 0;
43 + num = (den) ? num / den : 0;
44
45 power_supply_root->capacity->value = (unsigned long long)(num * 100.0);
46 }
@@ -75,7 +75,7 @@ static void netdata_power_supply_plot(struct simple_property *voltage, int updat
75
76 int do_GetPowerSupply(int update_every, usec_t dt __maybe_unused)
77 {
78 - static struct simple_property voltage;
78 + static struct simple_property voltage = {0};
79
80 HDEVINFO hdev = SetupDiGetClassDevs(&GUID_DEVCLASS_BATTERY, 0, 0, DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
81 if (hdev == INVALID_HANDLE_VALUE)
@@ -145,10 +145,13 @@ int do_GetPowerSupply(int update_every, usec_t dt __maybe_unused)
145 freez(power_supply_root->name);
146 if (likely(power_supply_root->capacity->filename))
147 freez(power_supply_root->capacity->filename);
148 + if (likely(voltage.filename))
149 + freez(voltage.filename);
150
149 - power_supply_root->name = power_supply_root->capacity->filename = NULL;
151 + power_supply_root->name = power_supply_root->capacity->filename = voltage.filename = NULL;
152 power_supply_root->name = strdupz(name);
153 power_supply_root->capacity->filename = strdupz(power_supply_root->name);
154 + voltage.filename = strdupz(power_supply_root->name);
155
156 netdata_update_power_supply_values(hBattery, &voltage, &bi, &bqi);
157