From 406901cb7016327b5cf1cdad38cd3b0f59a9956a Mon Sep 17 00:00:00 2001 From: Joachim Gabler Date: Fri, 31 Jan 2025 15:00:56 +0100 Subject: [PATCH] BF: CS-937 do full valgrind test on V90_BRANCH (9.0.2) // Fixed memory leaks in qstat, qconf, and qhost // Updated the valgrind suppressions file for unresolved issue CS-988 --- source/clients/common/ocs_client_job.cc | 18 +++++---------- source/clients/qconf/ocs_qconf_centry.cc | 28 +++++++++++++++++++----- source/clients/qconf/ocs_qconf_parse.cc | 6 ++--- source/clients/qhost/ocs_qhost_print.cc | 2 +- valgrind.supp | 15 +++++++++++++ 5 files changed, 46 insertions(+), 23 deletions(-) diff --git a/source/clients/common/ocs_client_job.cc b/source/clients/common/ocs_client_job.cc index 8e8bc391e..1fab13176 100644 --- a/source/clients/common/ocs_client_job.cc +++ b/source/clients/common/ocs_client_job.cc @@ -657,12 +657,12 @@ void cull_show_job(const lListElem *job, int flags, bool show_binding) { } { - dstring wallclock_string = DSTRING_INIT; - dstring cpu_string = DSTRING_INIT; - dstring vmem_string = DSTRING_INIT; - dstring maxvmem_string = DSTRING_INIT; - dstring rss_string = DSTRING_INIT; - dstring maxrss_string = DSTRING_INIT; + DSTRING_STATIC(wallclock_string, 32); + DSTRING_STATIC(cpu_string, 32); + DSTRING_STATIC(vmem_string, 32); + DSTRING_STATIC(maxvmem_string, 32); + DSTRING_STATIC(rss_string, 32); + DSTRING_STATIC(maxrss_string, 32); double_print_time_to_dstring(wallclock, &wallclock_string, true); double_print_time_to_dstring(cpu, &cpu_string, true); @@ -677,12 +677,6 @@ void cull_show_job(const lListElem *job, int flags, bool show_binding) { (maxvmem == 0.0) ? "N/A" : sge_dstring_get_string(&maxvmem_string), (rss == 0.0) ? "N/A" : sge_dstring_get_string(&rss_string), (maxrss == 0.0) ? "N/A" : sge_dstring_get_string(&maxrss_string)); - - sge_dstring_free(&cpu_string); - sge_dstring_free(&vmem_string); - sge_dstring_free(&maxvmem_string); - sge_dstring_free(&rss_string); - sge_dstring_free(&maxrss_string); } } } diff --git a/source/clients/qconf/ocs_qconf_centry.cc b/source/clients/qconf/ocs_qconf_centry.cc index 6143afbe2..ceeb79ac9 100644 --- a/source/clients/qconf/ocs_qconf_centry.cc +++ b/source/clients/qconf/ocs_qconf_centry.cc @@ -71,6 +71,8 @@ bool centry_add_del_mod_via_gdi(lListElem *this_elem, lList **answer_list, ocs:: lAppendElem(centry_list, this_elem); gdi_answer_list = ocs::gdi::Client::sge_gdi(ocs::gdi::Target::TargetValue::SGE_CE_LIST, gdi_command, ocs::gdi::SubCommand::SGE_GDI_SUB_NONE, ¢ry_list, nullptr, nullptr); + lDechainElem(centry_list, this_elem); + lFreeList(¢ry_list); answer_list_replace(answer_list, &gdi_answer_list); } @@ -78,9 +80,10 @@ bool centry_add_del_mod_via_gdi(lListElem *this_elem, lList **answer_list, ocs:: } lListElem *centry_get_via_gdi(lList **answer_list, const char *name) { + DENTER(TOP_LAYER); + lListElem *ret = nullptr; - DENTER(TOP_LAYER); if (name != nullptr) { lList *gdi_answer_list = nullptr; lEnumeration *what = nullptr; @@ -94,10 +97,13 @@ lListElem *centry_get_via_gdi(lList **answer_list, const char *name) { lFreeWhere(&where); if (!answer_list_has_error(&gdi_answer_list)) { - ret = lFirstRW(centry_list); + ret = lDechainElem(centry_list, lFirstRW(centry_list)); + lFreeList(&gdi_answer_list); } else { answer_list_replace(answer_list, &gdi_answer_list); } + + lFreeList(¢ry_list); } DRETURN(ret); @@ -169,9 +175,11 @@ static bool centry_provide_modify_context(lListElem **this_elem, lList **answer_ } bool centry_add(lList **answer_list, const char *name) { - bool ret = true; DENTER(TOP_LAYER); + + bool ret = true; + if (name != nullptr) { lListElem *centry = centry_create(answer_list, name); @@ -184,6 +192,7 @@ bool centry_add(lList **answer_list, const char *name) { if (ret) { ret &= centry_add_del_mod_via_gdi(centry, answer_list, ocs::gdi::Command::SGE_GDI_ADD); } + lFreeElem(¢ry); } DRETURN(ret); @@ -221,15 +230,18 @@ bool centry_add_from_file(lList **answer_list, const char *filename) { if (ret) { ret &= centry_add_del_mod_via_gdi(centry, answer_list, ocs::gdi::Command::SGE_GDI_ADD); } + + lFreeElem(¢ry); } DRETURN(ret); } bool centry_modify(lList **answer_list, const char *name) { + DENTER(TOP_LAYER); + bool ret = true; - DENTER(TOP_LAYER); if (name != nullptr) { lListElem *centry = centry_get_via_gdi(answer_list, name); @@ -243,7 +255,7 @@ bool centry_modify(lList **answer_list, const char *name) { if (ret) { ret &= centry_add_del_mod_via_gdi(centry, answer_list, ocs::gdi::Command::SGE_GDI_MOD); } - if (centry) { + if (centry != nullptr) { lFreeElem(¢ry); } } @@ -303,15 +315,18 @@ bool centry_delete(lList **answer_list, const char *name) { if (centry != nullptr) { ret &= centry_add_del_mod_via_gdi(centry, answer_list, ocs::gdi::Command::SGE_GDI_DEL); } + + lFreeElem(¢ry); } DRETURN(ret); } bool centry_show(lList **answer_list, const char *name) { + DENTER(TOP_LAYER); + bool ret = true; - DENTER(TOP_LAYER); if (name != nullptr) { lListElem *centry = centry_get_via_gdi(answer_list, name); @@ -329,6 +344,7 @@ bool centry_show(lList **answer_list, const char *name) { ret = false; } } + DRETURN(ret); } diff --git a/source/clients/qconf/ocs_qconf_parse.cc b/source/clients/qconf/ocs_qconf_parse.cc index d43c61f3d..330bf69d0 100644 --- a/source/clients/qconf/ocs_qconf_parse.cc +++ b/source/clients/qconf/ocs_qconf_parse.cc @@ -2228,7 +2228,6 @@ int sge_parse_qconf(char *argv[]) spp = sge_parser_get_next(spp); /* read file */ - lp = lCreateList("exechosts to change", EH_Type); fields_out[0] = NoName; ep = spool_flatfile_read_object(&alp, EH_Type, nullptr, fields, fields_out, true, &qconf_sfi, @@ -2255,8 +2254,6 @@ int sge_parse_qconf(char *argv[]) DRETURN(1); } - lAppendElem(lp, ep); - /* test host name */ if (sge_resolve_host(ep, EH_name) != CL_RETVAL_OK) { fprintf(stderr, MSG_SGETEXT_CANTRESOLVEHOST_S, lGetHost(ep, EH_name)); @@ -2266,6 +2263,8 @@ int sge_parse_qconf(char *argv[]) DRETURN(1); } + lp = lCreateList("exechosts to change", EH_Type); + lAppendElem(lp, ep); alp = ocs::gdi::Client::sge_gdi(ocs::gdi::Target::SGE_EH_LIST, ocs::gdi::Command::SGE_GDI_MOD, ocs::gdi::SubCommand::SGE_GDI_SUB_NONE, &lp, nullptr, nullptr); sge_parse_return |= show_answer(alp); @@ -4726,7 +4725,6 @@ int sge_parse_qconf(char *argv[]) qconf_is_manager(username); spp = sge_parser_get_next(spp); - qconf_is_manager(username); centry_add(&answer_list, *spp); sge_parse_return |= show_answer(answer_list); lFreeList(&answer_list); diff --git a/source/clients/qhost/ocs_qhost_print.cc b/source/clients/qhost/ocs_qhost_print.cc index a7018f09d..83502f05b 100644 --- a/source/clients/qhost/ocs_qhost_print.cc +++ b/source/clients/qhost/ocs_qhost_print.cc @@ -508,7 +508,7 @@ sge_print_host(lListElem *hep, lList *centry_list, lList *acl_list, qhost_report DRETURN(ret); } } else { - dstring output = DSTRING_INIT; + DSTRING_STATIC(output, 256); if (show_binding) { if (hide_data) { diff --git a/valgrind.supp b/valgrind.supp index 3a9ce5350..06f7f8625 100644 --- a/valgrind.supp +++ b/valgrind.supp @@ -203,3 +203,18 @@ fun:_Z11sge_jobnamePKc fun:main } + +{ + @todo CS-988 qsub sets up a signal thread but does never delete it again + Memcheck:Leak + match-leak-kinds: possible + fun:calloc + fun:calloc + fun:allocate_dtv + fun:_dl_allocate_tls + fun:allocate_stack + fun:pthread_create@@GLIBC_2.34 + fun:main +} + +