]> git.ipfire.org Git - thirdparty/collectd.git/commitdiff
SMART plugin: initialize struct passed to `ioctl(2)`. 4165/head
authorFlorian Forster <octo@collectd.org>
Sat, 25 Nov 2023 13:12:59 +0000 (14:12 +0100)
committerFlorian Forster <octo@collectd.org>
Sat, 25 Nov 2023 13:12:59 +0000 (14:12 +0100)
Valgrind is complaining about a conditional jump based on uninitialized
memory:

```
==66462== Conditional jump or move depends on uninitialised value(s)
==66462==    at 0x10C500: smart_read_nvme_intel_disk (in /__w/collectd/collectd/test_plugin_smart)
==66462==    by 0x10D366: test_x (in /__w/collectd/collectd/test_plugin_smart)
==66462==    by 0x10D638: main (in /__w/collectd/collectd/test_plugin_smart)
```

This may be due to the `struct nvme_additional_smart_log` being
uninitialized when it's being passed to `ioctl(2)`.

This there, this removed an unnecessary level of indentation.

src/smart.c

index 383b98693b5957f8402021f97b2bb01f1cc367a8..62d896e4c56bdff408b83670fe321fcc5fec9e4b 100644 (file)
@@ -383,13 +383,7 @@ static int smart_read_nvme_disk(const char *dev, char const *name) {
 }
 
 static int smart_read_nvme_intel_disk(const char *dev, char const *name) {
-
-  DEBUG("name = %s", name);
-  DEBUG("dev = %s", dev);
-
-  struct nvme_additional_smart_log intel_smart_log;
-  int fd, status;
-  fd = open(dev, O_RDWR);
+  int fd = open(dev, O_RDWR);
   if (fd < 0) {
     ERROR(PLUGIN_NAME ": open failed with %s\n", strerror(errno));
     return fd;
@@ -400,88 +394,86 @@ static int smart_read_nvme_intel_disk(const char *dev, char const *name) {
    * - Additional SMART Attributes (Log Identfiter CAh)
    */
 
-  status =
-      ioctl(fd, NVME_IOCTL_ADMIN_CMD,
-            &(struct nvme_admin_cmd){.opcode = NVME_ADMIN_GET_LOG_PAGE,
-                                     .nsid = NVME_NSID_ALL,
-                                     .addr = (unsigned long)&intel_smart_log,
-                                     .data_len = sizeof(intel_smart_log),
-                                     .cdw10 = NVME_SMART_INTEL_CDW10});
+  struct nvme_additional_smart_log intel_smart_log = {0};
+  int status = ioctl(fd, NVME_IOCTL_ADMIN_CMD,
+                     &(struct nvme_admin_cmd){
+                         .opcode = NVME_ADMIN_GET_LOG_PAGE,
+                         .nsid = NVME_NSID_ALL,
+                         .addr = (unsigned long)&intel_smart_log,
+                         .data_len = sizeof(intel_smart_log),
+                         .cdw10 = NVME_SMART_INTEL_CDW10,
+                     });
   if (status < 0) {
     ERROR(PLUGIN_NAME ": ioctl for NVME_IOCTL_ADMIN_CMD failed with %s\n",
           strerror(errno));
     close(fd);
     return status;
-  } else {
-
-    smart_submit(name, "nvme_program_fail_count", "norm",
-                 (double)intel_smart_log.program_fail_cnt.norm);
-    smart_submit(name, "nvme_program_fail_count", "raw",
-                 int48_to_double(intel_smart_log.program_fail_cnt.raw));
-    smart_submit(name, "nvme_erase_fail_count", "norm",
-                 (double)intel_smart_log.erase_fail_cnt.norm);
-    smart_submit(name, "nvme_erase_fail_count", "raw",
-                 int48_to_double(intel_smart_log.program_fail_cnt.raw));
-    smart_submit(name, "nvme_wear_leveling", "norm",
-                 (double)intel_smart_log.wear_leveling_cnt.norm);
-    smart_submit(
-        name, "nvme_wear_leveling", "min",
-        (double)le16_to_cpu(intel_smart_log.wear_leveling_cnt.wear_level.min));
-    smart_submit(
-        name, "nvme_wear_leveling", "max",
-        (double)le16_to_cpu(intel_smart_log.wear_leveling_cnt.wear_level.max));
-    smart_submit(
-        name, "nvme_wear_leveling", "avg",
-        (double)le16_to_cpu(intel_smart_log.wear_leveling_cnt.wear_level.avg));
-    smart_submit(name, "nvme_end_to_end_error_detection_count", "norm",
-                 (double)intel_smart_log.e2e_err_cnt.norm);
-    smart_submit(name, "nvme_end_to_end_error_detection_count", "raw",
-                 int48_to_double(intel_smart_log.e2e_err_cnt.raw));
-    smart_submit(name, "nvme_crc_error_count", "norm",
-                 (double)intel_smart_log.crc_err_cnt.norm);
-    smart_submit(name, "nvme_crc_error_count", "raw",
-                 int48_to_double(intel_smart_log.crc_err_cnt.raw));
-    smart_submit(name, "nvme_timed_workload_media_wear", "norm",
-                 (double)intel_smart_log.timed_workload_media_wear.norm);
-    smart_submit(
-        name, "nvme_timed_workload_media_wear", "raw",
-        int48_to_double(intel_smart_log.timed_workload_media_wear.raw));
-    smart_submit(name, "nvme_timed_workload_host_reads", "norm",
-                 (double)intel_smart_log.timed_workload_host_reads.norm);
-    smart_submit(
-        name, "nvme_timed_workload_host_reads", "raw",
-        int48_to_double(intel_smart_log.timed_workload_host_reads.raw));
-    smart_submit(name, "nvme_timed_workload_timer", "norm",
-                 (double)intel_smart_log.timed_workload_timer.norm);
-    smart_submit(name, "nvme_timed_workload_timer", "raw",
-                 int48_to_double(intel_smart_log.timed_workload_timer.raw));
-    smart_submit(name, "nvme_thermal_throttle_status", "norm",
-                 (double)intel_smart_log.thermal_throttle_status.norm);
-    smart_submit(
-        name, "nvme_thermal_throttle_status", "pct",
-        (double)intel_smart_log.thermal_throttle_status.thermal_throttle.pct);
-    smart_submit(
-        name, "nvme_thermal_throttle_status", "count",
-        (double)intel_smart_log.thermal_throttle_status.thermal_throttle.count);
-    smart_submit(name, "nvme_retry_buffer_overflow_count", "norm",
-                 (double)intel_smart_log.retry_buffer_overflow_cnt.norm);
-    smart_submit(
-        name, "nvme_retry_buffer_overflow_count", "raw",
-        int48_to_double(intel_smart_log.retry_buffer_overflow_cnt.raw));
-    smart_submit(name, "nvme_pll_lock_loss_count", "norm",
-                 (double)intel_smart_log.pll_lock_loss_cnt.norm);
-    smart_submit(name, "nvme_pll_lock_loss_count", "raw",
-                 int48_to_double(intel_smart_log.pll_lock_loss_cnt.raw));
-    smart_submit(name, "nvme_nand_bytes_written", "norm",
-                 (double)intel_smart_log.host_bytes_written.norm);
-    smart_submit(name, "nvme_nand_bytes_written", "raw",
-                 int48_to_double(intel_smart_log.host_bytes_written.raw));
-    smart_submit(name, "nvme_host_bytes_written", "norm",
-                 (double)intel_smart_log.host_bytes_written.norm);
-    smart_submit(name, "nvme_host_bytes_written", "raw",
-                 int48_to_double(intel_smart_log.host_bytes_written.raw));
   }
 
+  smart_submit(name, "nvme_program_fail_count", "norm",
+               (double)intel_smart_log.program_fail_cnt.norm);
+  smart_submit(name, "nvme_program_fail_count", "raw",
+               int48_to_double(intel_smart_log.program_fail_cnt.raw));
+  smart_submit(name, "nvme_erase_fail_count", "norm",
+               (double)intel_smart_log.erase_fail_cnt.norm);
+  smart_submit(name, "nvme_erase_fail_count", "raw",
+               int48_to_double(intel_smart_log.program_fail_cnt.raw));
+  smart_submit(name, "nvme_wear_leveling", "norm",
+               (double)intel_smart_log.wear_leveling_cnt.norm);
+  smart_submit(
+      name, "nvme_wear_leveling", "min",
+      (double)le16_to_cpu(intel_smart_log.wear_leveling_cnt.wear_level.min));
+  smart_submit(
+      name, "nvme_wear_leveling", "max",
+      (double)le16_to_cpu(intel_smart_log.wear_leveling_cnt.wear_level.max));
+  smart_submit(
+      name, "nvme_wear_leveling", "avg",
+      (double)le16_to_cpu(intel_smart_log.wear_leveling_cnt.wear_level.avg));
+  smart_submit(name, "nvme_end_to_end_error_detection_count", "norm",
+               (double)intel_smart_log.e2e_err_cnt.norm);
+  smart_submit(name, "nvme_end_to_end_error_detection_count", "raw",
+               int48_to_double(intel_smart_log.e2e_err_cnt.raw));
+  smart_submit(name, "nvme_crc_error_count", "norm",
+               (double)intel_smart_log.crc_err_cnt.norm);
+  smart_submit(name, "nvme_crc_error_count", "raw",
+               int48_to_double(intel_smart_log.crc_err_cnt.raw));
+  smart_submit(name, "nvme_timed_workload_media_wear", "norm",
+               (double)intel_smart_log.timed_workload_media_wear.norm);
+  smart_submit(name, "nvme_timed_workload_media_wear", "raw",
+               int48_to_double(intel_smart_log.timed_workload_media_wear.raw));
+  smart_submit(name, "nvme_timed_workload_host_reads", "norm",
+               (double)intel_smart_log.timed_workload_host_reads.norm);
+  smart_submit(name, "nvme_timed_workload_host_reads", "raw",
+               int48_to_double(intel_smart_log.timed_workload_host_reads.raw));
+  smart_submit(name, "nvme_timed_workload_timer", "norm",
+               (double)intel_smart_log.timed_workload_timer.norm);
+  smart_submit(name, "nvme_timed_workload_timer", "raw",
+               int48_to_double(intel_smart_log.timed_workload_timer.raw));
+  smart_submit(name, "nvme_thermal_throttle_status", "norm",
+               (double)intel_smart_log.thermal_throttle_status.norm);
+  smart_submit(
+      name, "nvme_thermal_throttle_status", "pct",
+      (double)intel_smart_log.thermal_throttle_status.thermal_throttle.pct);
+  smart_submit(
+      name, "nvme_thermal_throttle_status", "count",
+      (double)intel_smart_log.thermal_throttle_status.thermal_throttle.count);
+  smart_submit(name, "nvme_retry_buffer_overflow_count", "norm",
+               (double)intel_smart_log.retry_buffer_overflow_cnt.norm);
+  smart_submit(name, "nvme_retry_buffer_overflow_count", "raw",
+               int48_to_double(intel_smart_log.retry_buffer_overflow_cnt.raw));
+  smart_submit(name, "nvme_pll_lock_loss_count", "norm",
+               (double)intel_smart_log.pll_lock_loss_cnt.norm);
+  smart_submit(name, "nvme_pll_lock_loss_count", "raw",
+               int48_to_double(intel_smart_log.pll_lock_loss_cnt.raw));
+  smart_submit(name, "nvme_nand_bytes_written", "norm",
+               (double)intel_smart_log.host_bytes_written.norm);
+  smart_submit(name, "nvme_nand_bytes_written", "raw",
+               int48_to_double(intel_smart_log.host_bytes_written.raw));
+  smart_submit(name, "nvme_host_bytes_written", "norm",
+               (double)intel_smart_log.host_bytes_written.norm);
+  smart_submit(name, "nvme_host_bytes_written", "raw",
+               int48_to_double(intel_smart_log.host_bytes_written.raw));
+
   close(fd);
   return 0;
 }