[Mesa-dev] [PATCH 2/2] HUD: Add support for block I/O, network I/O and lmsensor stats

Steven Toth stoth at kernellabs.com
Tue Sep 27 23:05:33 UTC 2016


V5: Feedback based on peer review
 convert sprintf to snprintf
 convert char * to const char *
 int arg converted to bool
 Func changes to take a filename vs a larger struct.
 omit the space between '*' and the param name.

Signed-off-by: Steven Toth <stoth at kernellabs.com>
---
 src/gallium/auxiliary/hud/hud_diskstat.c     | 36 ++++++++++---------
 src/gallium/auxiliary/hud/hud_nic.c          | 53 +++++++++++++++-------------
 src/gallium/auxiliary/hud/hud_private.h      | 12 +++----
 src/gallium/auxiliary/hud/hud_sensors_temp.c | 23 ++++++------
 4 files changed, 66 insertions(+), 58 deletions(-)

diff --git a/src/gallium/auxiliary/hud/hud_diskstat.c b/src/gallium/auxiliary/hud/hud_diskstat.c
index d22afb7..b2ee1f8 100644
--- a/src/gallium/auxiliary/hud/hud_diskstat.c
+++ b/src/gallium/auxiliary/hud/hud_diskstat.c
@@ -85,7 +85,7 @@ static int gdiskstat_count = 0;
 static struct list_head gdiskstat_list;
 
 static struct diskstat_info *
-find_dsi_by_name(char *n, int mode)
+find_dsi_by_name(const char *n, int mode)
 {
    list_for_each_entry(struct diskstat_info, dsi, &gdiskstat_list, list) {
       if (dsi->mode != mode)
@@ -97,10 +97,10 @@ find_dsi_by_name(char *n, int mode)
 }
 
 static int
-get_file_values(struct diskstat_info *dsi, struct stat_s *s)
+get_file_values(char *fn, struct stat_s *s)
 {
    int ret = 0;
-   FILE *fh = fopen(dsi->sysfs_filename, "r");
+   FILE *fh = fopen(fn, "r");
    if (!fh)
       return -1;
 
@@ -128,7 +128,7 @@ query_dsi_load(struct hud_graph *gr)
    if (dsi->last_time) {
       if (dsi->last_time + gr->pane->period <= now) {
          struct stat_s stat;
-         if (get_file_values(dsi, &stat) < 0)
+         if (get_file_values(dsi->sysfs_filename, &stat) < 0)
             return;
          float val = 0;
 
@@ -157,7 +157,7 @@ query_dsi_load(struct hud_graph *gr)
       switch (dsi->mode) {
       case DISKSTAT_RD:
       case DISKSTAT_WR:
-         get_file_values(dsi, &dsi->last_stat);
+         get_file_values(dsi->sysfs_filename, &dsi->last_stat);
          break;
       }
       dsi->last_time = now;
@@ -179,7 +179,7 @@ free_query_data(void *p)
   * \param  mode  query read or write (DISKSTAT_RD/DISKSTAT_WR) statistics.
   */
 void
-hud_diskstat_graph_install(struct hud_pane *pane, char *dev_name,
+hud_diskstat_graph_install(struct hud_pane *pane, const char *dev_name,
                            unsigned int mode)
 {
    struct hud_graph *gr;
@@ -205,10 +205,10 @@ hud_diskstat_graph_install(struct hud_pane *pane, char *dev_name,
 
    dsi->mode = mode;
    if (dsi->mode == DISKSTAT_RD) {
-      sprintf(gr->name, "%s-Read-MB/s", dsi->name);
+      snprintf(gr->name, sizeof(gr->name), "%s-Read-MB/s", dsi->name);
    }
    else if (dsi->mode == DISKSTAT_WR) {
-      sprintf(gr->name, "%s-Write-MB/s", dsi->name);
+      snprintf(gr->name, sizeof(gr->name), "%s-Write-MB/s", dsi->name);
    }
    else
       return;
@@ -226,24 +226,26 @@ hud_diskstat_graph_install(struct hud_pane *pane, char *dev_name,
 }
 
 static void
-add_object_part(char *basename, char *name, int objmode)
+add_object_part(const char *basename, char *name, int objmode)
 {
    struct diskstat_info *dsi = CALLOC_STRUCT(diskstat_info);
 
    strcpy(dsi->name, name);
-   sprintf(dsi->sysfs_filename, "%s/%s/stat", basename, name);
+   snprintf(dsi->sysfs_filename, sizeof(dsi->sysfs_filename), "%s/%s/stat",
+      basename, name);
    dsi->mode = objmode;
    list_addtail(&dsi->list, &gdiskstat_list);
    gdiskstat_count++;
 }
 
 static void
-add_object(char *basename, char *name, int objmode)
+add_object(const char *basename, char *name, int objmode)
 {
    struct diskstat_info *dsi = CALLOC_STRUCT(diskstat_info);
 
    strcpy(dsi->name, name);
-   sprintf(dsi->sysfs_filename, "%s/stat", basename);
+   snprintf(dsi->sysfs_filename, sizeof(dsi->sysfs_filename), "%s/stat",
+      basename);
    dsi->mode = objmode;
    list_addtail(&dsi->list, &gdiskstat_list);
    gdiskstat_count++;
@@ -256,7 +258,7 @@ add_object(char *basename, char *name, int objmode)
   * \return  number of detected block I/O devices.
   */
 int
-hud_get_num_disks(int displayhelp)
+hud_get_num_disks(bool displayhelp)
 {
    struct dirent *dp;
    struct stat stat_buf;
@@ -281,8 +283,8 @@ hud_get_num_disks(int displayhelp)
          continue;
 
       char basename[256];
-      sprintf(basename, "/sys/block/%s", dp->d_name);
-      sprintf(name, "%s/stat", basename);
+      snprintf(basename, sizeof(basename), "/sys/block/%s", dp->d_name);
+      snprintf(name, sizeof(name), "%s/stat", basename);
       if (stat(name, &stat_buf) < 0)
          continue;
 
@@ -305,7 +307,7 @@ hud_get_num_disks(int displayhelp)
             continue;
 
          char p[64];
-         sprintf(p, "%s/%s/stat", basename, dpart->d_name);
+         snprintf(p, sizeof(p), "%s/%s/stat", basename, dpart->d_name);
          if (stat(p, &stat_buf) < 0)
             continue;
 
@@ -321,7 +323,7 @@ hud_get_num_disks(int displayhelp)
    if (displayhelp) {
       list_for_each_entry(struct diskstat_info, dsi, &gdiskstat_list, list) {
          char line[32];
-         sprintf(line, "    diskstat-%s-%s",
+         snprintf(line, sizeof(line), "    diskstat-%s-%s",
                  dsi->mode == DISKSTAT_RD ? "rd" :
                  dsi->mode == DISKSTAT_WR ? "wr" : "undefined", dsi->name);
 
diff --git a/src/gallium/auxiliary/hud/hud_nic.c b/src/gallium/auxiliary/hud/hud_nic.c
index 0d35031..c343926 100644
--- a/src/gallium/auxiliary/hud/hud_nic.c
+++ b/src/gallium/auxiliary/hud/hud_nic.c
@@ -70,7 +70,7 @@ static int gnic_count = 0;
 static struct list_head gnic_list;
 
 static struct nic_info *
-find_nic_by_name(char *n, int mode)
+find_nic_by_name(const char *n, int mode)
 {
    list_for_each_entry(struct nic_info, nic, &gnic_list, list) {
       if (nic->mode != mode)
@@ -83,7 +83,7 @@ find_nic_by_name(char *n, int mode)
 }
 
 static int
-get_file_value(char *fname, uint64_t * value)
+get_file_value(char *fname, uint64_t *value)
 {
    FILE *fh = fopen(fname, "r");
    if (!fh)
@@ -96,16 +96,16 @@ get_file_value(char *fname, uint64_t * value)
 }
 
 static boolean
-get_nic_bytes(struct nic_info *nic, uint64_t * bytes)
+get_nic_bytes(char *fn, uint64_t *bytes)
 {
-   if (get_file_value(nic->throughput_filename, bytes) < 0)
+   if (get_file_value(fn, bytes) < 0)
       return FALSE;
 
    return TRUE;
 }
 
 static void
-query_wifi_bitrate(struct nic_info *nic, uint64_t * bitrate)
+query_wifi_bitrate(struct nic_info *nic, uint64_t *bitrate)
 {
    int sockfd;
    struct iw_statistics stats;
@@ -136,7 +136,7 @@ query_wifi_bitrate(struct nic_info *nic, uint64_t * bitrate)
 }
 
 static void
-query_nic_rssi(struct nic_info *nic, uint64_t * leveldBm)
+query_nic_rssi(struct nic_info *nic, uint64_t *leveldBm)
 {
    int sockfd;
    struct iw_statistics stats;
@@ -194,7 +194,7 @@ query_nic_load(struct hud_graph *gr)
          case NIC_DIRECTION_TX:
             {
                uint64_t bytes;
-               get_nic_bytes(nic, &bytes);
+               get_nic_bytes(nic->throughput_filename, &bytes);
                uint64_t nic_mbps =
                   ((bytes - nic->last_nic_bytes) / 1000000) * 8;
 
@@ -233,7 +233,7 @@ query_nic_load(struct hud_graph *gr)
       switch (nic->mode) {
       case NIC_DIRECTION_RX:
       case NIC_DIRECTION_TX:
-         get_nic_bytes(nic, &nic->last_nic_bytes);
+         get_nic_bytes(nic->throughput_filename, &nic->last_nic_bytes);
          break;
       case NIC_RSSI_DBM:
          break;
@@ -258,7 +258,7 @@ free_query_data(void *p)
   * \param  mode  query type (NIC_DIRECTION_RX/WR/RSSI) statistics.
   */
 void
-hud_nic_graph_install(struct hud_pane *pane, char *nic_name,
+hud_nic_graph_install(struct hud_pane *pane, const char *nic_name,
                       unsigned int mode)
 {
    struct hud_graph *gr;
@@ -285,13 +285,15 @@ hud_nic_graph_install(struct hud_pane *pane, char *nic_name,
 
    nic->mode = mode;
    if (nic->mode == NIC_DIRECTION_RX) {
-      sprintf(gr->name, "%s-rx-%lldMbps", nic->name, nic->speedMbps);
+      snprintf(gr->name, sizeof(gr->name), "%s-rx-%lldMbps", nic->name,
+         nic->speedMbps);
    }
    else if (nic->mode == NIC_DIRECTION_TX) {
-      sprintf(gr->name, "%s-tx-%lldMbps", nic->name, nic->speedMbps);
+      snprintf(gr->name, sizeof(gr->name), "%s-tx-%lldMbps", nic->name,
+         nic->speedMbps);
    }
    else if (nic->mode == NIC_RSSI_DBM)
-      sprintf(gr->name, "%s-rssi", nic->name);
+      snprintf(gr->name, sizeof(gr->name), "%s-rssi", nic->name);
    else
       return;
 
@@ -314,7 +316,7 @@ is_wireless_nic(char *dirbase)
 
    /* Check if its a wireless card */
    char fn[256];
-   sprintf(fn, "%s/wireless", dirbase);
+   snprintf(fn, sizeof(fn), "%s/wireless", dirbase);
    if (stat(fn, &stat_buf) == 0)
       return 1;
 
@@ -328,7 +330,7 @@ query_nic_bitrate(struct nic_info *nic, char *dirbase)
 
    /* Check if its a wireless card */
    char fn[256];
-   sprintf(fn, "%s/wireless", dirbase);
+   snprintf(fn, sizeof(fn), "%s/wireless", dirbase);
    if (stat(fn, &stat_buf) == 0) {
       /* we're a wireless nic */
       query_wifi_bitrate(nic, &nic->speedMbps);
@@ -336,7 +338,7 @@ query_nic_bitrate(struct nic_info *nic, char *dirbase)
    }
    else {
       /* Must be a wired nic */
-      sprintf(fn, "%s/speed", dirbase);
+      snprintf(fn, sizeof(fn), "%s/speed", dirbase);
       get_file_value(fn, &nic->speedMbps);
    }
 }
@@ -348,7 +350,7 @@ query_nic_bitrate(struct nic_info *nic, char *dirbase)
   * \return  number of detected network interface devices.
   */
 int
-hud_get_num_nics(int displayhelp)
+hud_get_num_nics(bool displayhelp)
 {
    struct dirent *dp;
    struct stat stat_buf;
@@ -374,8 +376,8 @@ hud_get_num_nics(int displayhelp)
          continue;
 
       char basename[256];
-      sprintf(basename, "/sys/class/net/%s", dp->d_name);
-      sprintf(name, "%s/statistics/rx_bytes", basename);
+      snprintf(basename, sizeof(basename), "/sys/class/net/%s", dp->d_name);
+      snprintf(name, sizeof(name), "%s/statistics/rx_bytes", basename);
       if (stat(name, &stat_buf) < 0)
          continue;
 
@@ -387,7 +389,8 @@ hud_get_num_nics(int displayhelp)
       /* Add the RX object */
       nic = CALLOC_STRUCT(nic_info);
       strcpy(nic->name, dp->d_name);
-      sprintf(nic->throughput_filename, "%s/statistics/rx_bytes", basename);
+      snprintf(nic->throughput_filename, sizeof(nic->throughput_filename),
+         "%s/statistics/rx_bytes", basename);
       nic->mode = NIC_DIRECTION_RX;
       nic->is_wireless = is_wireless;
       query_nic_bitrate(nic, basename);
@@ -398,8 +401,9 @@ hud_get_num_nics(int displayhelp)
       /* Add the TX object */
       nic = CALLOC_STRUCT(nic_info);
       strcpy(nic->name, dp->d_name);
-      sprintf(nic->throughput_filename,
-              "/sys/class/net/%s/statistics/tx_bytes", dp->d_name);
+      snprintf(nic->throughput_filename,
+         sizeof(nic->throughput_filename),
+         "/sys/class/net/%s/statistics/tx_bytes", dp->d_name);
       nic->mode = NIC_DIRECTION_TX;
       nic->is_wireless = is_wireless;
 
@@ -412,8 +416,9 @@ hud_get_num_nics(int displayhelp)
          /* RSSI Support */
          nic = CALLOC_STRUCT(nic_info);
          strcpy(nic->name, dp->d_name);
-         sprintf(nic->throughput_filename,
-                 "/sys/class/net/%s/statistics/tx_bytes", dp->d_name);
+         snprintf(nic->throughput_filename,
+            sizeof(nic->throughput_filename),
+            "/sys/class/net/%s/statistics/tx_bytes", dp->d_name);
          nic->mode = NIC_RSSI_DBM;
 
          query_nic_bitrate(nic, basename);
@@ -426,7 +431,7 @@ hud_get_num_nics(int displayhelp)
 
    list_for_each_entry(struct nic_info, nic, &gnic_list, list) {
       char line[64];
-      sprintf(line, "    nic-%s-%s",
+      snprintf(line, sizeof(line), "    nic-%s-%s",
               nic->mode == NIC_DIRECTION_RX ? "rx" :
               nic->mode == NIC_DIRECTION_TX ? "tx" :
               nic->mode == NIC_RSSI_DBM ? "rssi" : "undefined", nic->name);
diff --git a/src/gallium/auxiliary/hud/hud_private.h b/src/gallium/auxiliary/hud/hud_private.h
index 5547fb6..c825512 100644
--- a/src/gallium/auxiliary/hud/hud_private.h
+++ b/src/gallium/auxiliary/hud/hud_private.h
@@ -104,27 +104,27 @@ void hud_batch_query_update(struct hud_batch_query_context *bq);
 void hud_batch_query_cleanup(struct hud_batch_query_context **pbq);
 
 #if HAVE_GALLIUM_EXTRA_HUD
-int hud_get_num_nics(int displayhelp);
+int hud_get_num_nics(bool displayhelp);
 #define NIC_DIRECTION_RX 1
 #define NIC_DIRECTION_TX 2
 #define NIC_RSSI_DBM     3
-void hud_nic_graph_install(struct hud_pane *pane, char *nic_index,
+void hud_nic_graph_install(struct hud_pane *pane, const char *nic_index,
                            unsigned int mode);
 
-int hud_get_num_disks(int displayhelp);
+int hud_get_num_disks(bool displayhelp);
 #define DISKSTAT_RD 1
 #define DISKSTAT_WR 2
-void hud_diskstat_graph_install(struct hud_pane *pane, char *dev_name,
+void hud_diskstat_graph_install(struct hud_pane *pane, const char *dev_name,
                                 unsigned int mode);
 #endif
 
 #if HAVE_LIBSENSORS
-int hud_get_num_sensors(int displayhelp);
+int hud_get_num_sensors(bool displayhelp);
 #define SENSORS_TEMP_CURRENT     1
 #define SENSORS_TEMP_CRITICAL    2
 #define SENSORS_VOLTAGE_CURRENT  3
 #define SENSORS_CURRENT_CURRENT  4
-void hud_sensors_temp_graph_install(struct hud_pane *pane, char *dev_name,
+void hud_sensors_temp_graph_install(struct hud_pane *pane, const char *dev_name,
                                     unsigned int mode);
 #endif
 
diff --git a/src/gallium/auxiliary/hud/hud_sensors_temp.c b/src/gallium/auxiliary/hud/hud_sensors_temp.c
index 425a16f..9f6d7c1 100644
--- a/src/gallium/auxiliary/hud/hud_sensors_temp.c
+++ b/src/gallium/auxiliary/hud/hud_sensors_temp.c
@@ -73,7 +73,7 @@ struct sensors_temp_info
 };
 
 static double
-get_value(const sensors_chip_name * name, const sensors_subfeature * sub)
+get_value(const sensors_chip_name *name, const sensors_subfeature *sub)
 {
    double val;
    int err;
@@ -140,7 +140,7 @@ get_sensor_values(struct sensors_temp_info *sti)
 }
 
 static struct sensors_temp_info *
-find_sti_by_name(char *n, unsigned int mode)
+find_sti_by_name(const char *n, unsigned int mode)
 {
    list_for_each_entry(struct sensors_temp_info, sti, &gsensors_temp_list, list) {
       if (sti->mode != mode)
@@ -204,7 +204,7 @@ free_query_data(void *p)
   * \param  mode  query type (NIC_DIRECTION_RX/WR/RSSI) statistics.
   */
 void
-hud_sensors_temp_graph_install(struct hud_pane *pane, char *dev_name,
+hud_sensors_temp_graph_install(struct hud_pane *pane, const char *dev_name,
                                unsigned int mode)
 {
    struct hud_graph *gr;
@@ -229,7 +229,7 @@ hud_sensors_temp_graph_install(struct hud_pane *pane, char *dev_name,
    if (!gr)
       return;
 
-   sprintf(gr->name, "%.6s..%s (%s)",
+   snprintf(gr->name, sizeof(gr->name), "%.6s..%s (%s)",
            sti->chipname,
            sti->featurename,
            sti->mode == SENSORS_VOLTAGE_CURRENT ? "Volts" :
@@ -262,7 +262,7 @@ hud_sensors_temp_graph_install(struct hud_pane *pane, char *dev_name,
 
 static void
 create_object(char *chipname, char *featurename,
-             const sensors_chip_name * chip, const sensors_feature * feature,
+             const sensors_chip_name *chip, const sensors_feature *feature,
              int mode)
 {
 #if LOCAL_DEBUG
@@ -275,7 +275,8 @@ create_object(char *chipname, char *featurename,
    sti->feature = feature;
    strcpy(sti->chipname, chipname);
    strcpy(sti->featurename, featurename);
-   sprintf(sti->name, "%s.%s", sti->chipname, sti->featurename);
+   snprintf(sti->name, sizeof(sti->name), "%s.%s", sti->chipname,
+      sti->featurename);
 
    list_addtail(&sti->list, &gsensors_temp_list);
    gsensors_temp_count++;
@@ -329,7 +330,7 @@ build_sensor_list(void)
   * \return  number of detected lmsensor devices.
   */
 int
-hud_get_num_sensors(int displayhelp)
+hud_get_num_sensors(bool displayhelp)
 {
    /* Return the number of sensors detected. */
    if (gsensors_temp_count)
@@ -351,16 +352,16 @@ hud_get_num_sensors(int displayhelp)
          char line[64];
          switch (sti->mode) {
          case SENSORS_TEMP_CURRENT:
-            sprintf(line, "    sensors_temp_cu-%s", sti->name);
+            snprintf(line, sizeof(line), "    sensors_temp_cu-%s", sti->name);
             break;
          case SENSORS_TEMP_CRITICAL:
-            sprintf(line, "    sensors_temp_cr-%s", sti->name);
+            snprintf(line, sizeof(line), "    sensors_temp_cr-%s", sti->name);
             break;
          case SENSORS_VOLTAGE_CURRENT:
-            sprintf(line, "    sensors_volt_cu-%s", sti->name);
+            snprintf(line, sizeof(line), "    sensors_volt_cu-%s", sti->name);
             break;
          case SENSORS_CURRENT_CURRENT:
-            sprintf(line, "    sensors_curr_cu-%s", sti->name);
+            snprintf(line, sizeof(line), "    sensors_curr_cu-%s", sti->name);
             break;
          }
 
-- 
2.7.4



More information about the mesa-dev mailing list