Skip to content

Commit

Permalink
BF: CS-937 do full valgrind test on V90_BRANCH (9.0.2)
Browse files Browse the repository at this point in the history 
// Fixed memory leaks in qstat, qconf, and qhost
// Updated the valgrind suppressions file for unresolved issue CS-988
  • Loading branch information
@jgabler-hpc
jgabler-hpc committed Jan 31, 2025
1 parent a9d4547 commit 406901c
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 23 deletions.
18 changes: 6 additions & 12 deletions source/clients/common/ocs_client_job.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
}
}
Expand Down
28 changes: 22 additions & 6 deletions source/clients/qconf/ocs_qconf_centry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,19 @@ 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,
&centry_list, nullptr, nullptr);
lDechainElem(centry_list, this_elem);
lFreeList(&centry_list);
answer_list_replace(answer_list, &gdi_answer_list);
}

DRETURN(ret);
}

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;
Expand All @@ -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(&centry_list);
}

DRETURN(ret);
Expand Down Expand Up @@ -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);

Expand All @@ -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(&centry);
}

DRETURN(ret);
Expand Down Expand Up @@ -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(&centry);
}

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);

Expand All @@ -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(&centry);
}
}
Expand Down Expand Up @@ -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(&centry);
}

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);

Expand All @@ -329,6 +344,7 @@ bool centry_show(lList **answer_list, const char *name) {
ret = false;
}
}

DRETURN(ret);
}

Expand Down
6 changes: 2 additions & 4 deletions source/clients/qconf/ocs_qconf_parse.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion source/clients/qhost/ocs_qhost_print.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
15 changes: 15 additions & 0 deletions valgrind.supp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}


0 comments on commit 406901c

Please sign in to comment.