diff options
author | Dimitris P <Dim-P@users.noreply.github.com> | 2023-08-08 18:52:46 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-08 20:52:46 +0300 |
commit | 56289bdc901c84f62139ec012309cf0ce9a3f9ca (patch) | |
tree | 5f96b9d6bcebfad600e06bc975ec8d3ca6ea554f /collectors/proc.plugin | |
parent | 4093cac2fab29bffe3faf04c54e6960b128176e6 (diff) |
Fix coverity issues introduced via drm proc module (#15765)
Diffstat (limited to 'collectors/proc.plugin')
-rw-r--r-- | collectors/proc.plugin/sys_class_drm.c | 72 |
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( |