Fix coverity issues introduced via drm proc module (#15765)

1 files changed, 41 insertions, 31 deletions

diff --git a/collectors/proc.plugin/sys_class_drm.c b/collectors/proc.plugin/sys_class_drm.c
index de38e9009d..284662cf65 100644
--- a/collectors/proc.plugin/sys_class_drm.c
+++ b/collectors/proc.plugin/sys_class_drm.c
@@ -7,8 +7,8 @@
 #define AMDGPU_CHART_TYPE "amdgpu"
 
 struct amdgpu_id_struct {
-    uint32_t asic_id;
-    uint32_t pci_rev_id;
+    unsigned long long asic_id;
+    unsigned long long pci_rev_id;
     const char *marketing_name;
 };
 
@@ -27,7 +27,7 @@ struct amdgpu_id_struct {
  * **IMPORTANT**: The amdgpu_ids has to be modified after new GPU releases. 
  * ------------------------------------------------------------------------*/
 
-static struct amdgpu_id_struct amdgpu_ids[] = {
+static const struct amdgpu_id_struct amdgpu_ids[] = {
     {0x1309, 0x00, "AMD Radeon R7 Graphics"},
     {0x130A, 0x00, "AMD Radeon R6 Graphics"},
     {0x130B, 0x00, "AMD Radeon R4 Graphics"},
@@ -615,34 +615,46 @@ static void card_free(struct card *c){
 }
 
 static int check_card_is_amdgpu(const char *const pathname){
-    procfile *const ff = procfile_open(pathname, " ", PROCFILE_FLAG_NO_ERROR_ON_FILE_IO);
-    if(unlikely(!procfile_readall(ff))){
-        procfile_close(ff);
-        return 1; // error
+    int rc = -1;
+
+    procfile *ff = procfile_open(pathname, " ", PROCFILE_FLAG_NO_ERROR_ON_FILE_IO);
+    if(unlikely(!ff)){
+        rc = -1;
+        goto cleanup;
+    } 
+
+    ff = procfile_readall(ff);
+    if(unlikely(!ff || procfile_lines(ff) < 1 || procfile_linewords(ff, 0) < 1)){
+        rc = -2;
+        goto cleanup;
     }
 
     for(size_t l = 0; l < procfile_lines(ff); l++) {
         if(!strcmp(procfile_lineword(ff, l, 0), "DRIVER=amdgpu")){
-            procfile_close(ff);
-            return 0;
+            rc = 0;
+            goto cleanup;
         }
     }
 
+    rc = -3; // no match
+
+cleanup:
     procfile_close(ff);
-    return 2; // no match
+    return rc;
 }
 
-static int read_multiline_file(procfile *ff, const char *const pathname, collected_number *num){
-    if(!ff) ff = procfile_open(pathname, NULL, PROCFILE_FLAG_DEFAULT);
-    if(unlikely(!procfile_readall(ff))){
-        procfile_close(ff);
-        return 1; // error
+static int read_clk_freq_file(procfile **p_ff, const char *const pathname, collected_number *num){
+    if(unlikely(!*p_ff)){
+        *p_ff = procfile_open(pathname, NULL, PROCFILE_FLAG_NO_ERROR_ON_FILE_IO);
+        if(unlikely(!*p_ff)) return -2;
     }
+    
+    if(unlikely(NULL == (*p_ff = procfile_readall(*p_ff)))) return -3;
 
-    for(size_t l = 0; l < procfile_lines(ff) ; l++) {
+    for(size_t l = 0; l < procfile_lines(*p_ff) ; l++) {
 
-        if(ff->lines->lines[l].words >= 3 && !strcmp(procfile_lineword(ff, l, 2), "*")){
-            char *str_with_units = procfile_lineword(ff, l, 1);
+        if((*p_ff)->lines->lines[l].words >= 3 && !strcmp(procfile_lineword((*p_ff), l, 2), "*")){
+            char *str_with_units = procfile_lineword((*p_ff), l, 1);
             char *delim = strchr(str_with_units, 'M');
             char str_without_units[10];
             memcpy(str_without_units, str_with_units, delim - str_with_units);
@@ -651,8 +663,8 @@ static int read_multiline_file(procfile *ff, const char *const pathname, collect
         }
     }
 
-    procfile_close(ff);
-    return 2; // error
+    procfile_close((*p_ff));
+    return -4;
 }
 
 static char *set_id(const char *const suf_1, const char *const suf_2, const char *const suf_3){
@@ -715,7 +727,7 @@ static int do_rrd_util_mem(struct card *const c){
 }
 
 static int do_rrd_clk_gpu(struct card *const c){
-    if(likely(!read_multiline_file(c->ff_clk_gpu, (char *) c->pathname_clk_gpu, &c->clk_gpu))){
+    if(likely(!read_clk_freq_file(&c->ff_clk_gpu, (char *) c->pathname_clk_gpu, &c->clk_gpu))){
         rrddim_set_by_pointer(c->st_clk_gpu, c->rd_clk_gpu, c->clk_gpu);
         rrdset_done(c->st_clk_gpu);
         return 0;
@@ -724,13 +736,12 @@ static int do_rrd_clk_gpu(struct card *const c){
         collector_error("Cannot read clk_gpu for %s: [%s]", c->pathname, c->id.marketing_name);
         freez((void *) c->pathname_clk_gpu);
         rrdset_is_obsolete(c->st_clk_gpu);
-        procfile_close(c->ff_clk_gpu);
         return 1;
     }
 }
 
 static int do_rrd_clk_mem(struct card *const c){
-    if(likely(!read_multiline_file(c->ff_clk_mem, (char *) c->pathname_clk_mem, &c->clk_mem))){
+    if(likely(!read_clk_freq_file(&c->ff_clk_mem, (char *) c->pathname_clk_mem, &c->clk_mem))){
         rrddim_set_by_pointer(c->st_clk_mem, c->rd_clk_mem, c->clk_mem);
         rrdset_done(c->st_clk_mem);
         return 0;
@@ -739,7 +750,6 @@ static int do_rrd_clk_mem(struct card *const c){
         collector_error("Cannot read clk_mem for %s: [%s]", c->pathname, c->id.marketing_name);
         freez((void *) c->pathname_clk_mem);
         rrdset_is_obsolete(c->st_clk_mem);
-        procfile_close(c->ff_clk_mem);
         return 1;
     }
 }
@@ -847,14 +857,14 @@ int do_sys_class_drm(int update_every, usec_t dt) {
                 c->pathname = strdupz(filename);
 
                 snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, "device/device");
-                if(read_single_base64_or_hex_number_file(filename, (unsigned long long *) &c->id.asic_id)){
+                if(read_single_base64_or_hex_number_file(filename, &c->id.asic_id)){
                     collector_error("Cannot read asic_id from '%s'", filename);
                     card_free(c);
                     continue;
                 }
 
                 snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, "device/revision");
-                if(read_single_base64_or_hex_number_file(filename, (unsigned long long *) &c->id.pci_rev_id)){
+                if(read_single_base64_or_hex_number_file(filename, &c->id.pci_rev_id)){
                     collector_error("Cannot read pci_rev_id from '%s'", filename);
                     card_free(c);
                     continue;
@@ -871,13 +881,13 @@ int do_sys_class_drm(int update_every, usec_t dt) {
 
 
                 collected_number tmp_val; 
-                #define set_prop_pathname(prop_filename, prop_pathname, ff){                    \
+                #define set_prop_pathname(prop_filename, prop_pathname, p_ff){                  \
                     snprintfz(filename, FILENAME_MAX, "%s/%s", c->pathname, prop_filename);     \
-                    if((ff && !read_multiline_file(ff, filename, &tmp_val)) ||                  \
+                    if((p_ff && !read_clk_freq_file(p_ff, filename, &tmp_val)) ||               \
                           !read_single_number_file(filename, (unsigned long long *) &tmp_val))  \
                         prop_pathname = strdupz(filename);                                      \
                     else                                                                        \
-                        collector_info("Cannot read file '%s'", filename);                     \
+                        collector_info("Cannot read file '%s'", filename);                      \
                 }
 
                 /* Initialize GPU and VRAM utilization metrics */
@@ -935,7 +945,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
 
                 /* Initialize GPU and VRAM clock frequency metrics */
 
-                set_prop_pathname("device/pp_dpm_sclk", c->pathname_clk_gpu, c->ff_clk_gpu);
+                set_prop_pathname("device/pp_dpm_sclk", c->pathname_clk_gpu, &c->ff_clk_gpu);
                 
                 if(c->pathname_clk_gpu){
                     c->st_clk_gpu = rrdset_create_localhost(
@@ -961,7 +971,7 @@ int do_sys_class_drm(int update_every, usec_t dt) {
 
                 }
 
-                set_prop_pathname("device/pp_dpm_mclk", c->pathname_clk_mem, c->ff_clk_mem);
+                set_prop_pathname("device/pp_dpm_mclk", c->pathname_clk_mem, &c->ff_clk_mem);
 
                 if(c->pathname_clk_mem){
                     c->st_clk_mem = rrdset_create_localhost(