From fa0af103d018d6dd783634cdfdc3ae3d5fc4632c Mon Sep 17 00:00:00 2001 From: Morris Jette <jette@schedmd.com> Date: Tue, 3 Sep 2013 11:15:13 -0700 Subject: [PATCH] Fix some possibly NULL pointers reported by CLANG None of these have ever occurred, but these changes will harden Slurm bug 403 --- .../jobcomp/filetxt/filetxt_jobcomp_process.c | 20 +++++++++---------- .../multifactor/priority_multifactor.c | 15 ++++++++------ src/slurmctld/step_mgr.c | 4 +++- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/plugins/jobcomp/filetxt/filetxt_jobcomp_process.c b/src/plugins/jobcomp/filetxt/filetxt_jobcomp_process.c index ac51f3a3ba2..660bff5e768 100644 --- a/src/plugins/jobcomp/filetxt/filetxt_jobcomp_process.c +++ b/src/plugins/jobcomp/filetxt/filetxt_jobcomp_process.c @@ -44,8 +44,8 @@ #include <ctype.h> #include <sys/stat.h> -#include "src/common/xmalloc.h" #include "src/common/slurm_jobcomp.h" +#include "src/common/xmalloc.h" #include "filetxt_jobcomp_process.h" #define BUFFER_SIZE 4096 @@ -106,30 +106,28 @@ static jobcomp_job_rec_t *_parse_line(List job_info_list) job->end_time = xstrdup(jobcomp_info->val); } else if (!strcasecmp("Userid", jobcomp_info->name)) { temp = strstr(jobcomp_info->val, "("); - if (!temp) - job->uid = atoi(jobcomp_info->val); - *temp++ = 0; - temp2 = temp; - temp = strstr(temp, ")"); if (!temp) { + job->uid = atoi(jobcomp_info->val); error("problem getting correct uid from %s", jobcomp_info->val); } else { + *temp++ = 0; + temp2 = temp; + temp = strstr(temp, ")"); *temp = 0; job->uid = atoi(temp2); job->uid_name = xstrdup(jobcomp_info->val); } } else if (!strcasecmp("GroupId", jobcomp_info->name)) { temp = strstr(jobcomp_info->val, "("); - if (!temp) - job->gid = atoi(jobcomp_info->val); - *temp++ = 0; - temp2 = temp; - temp = strstr(temp, ")"); if (!temp) { + job->gid = atoi(jobcomp_info->val); error("problem getting correct gid from %s", jobcomp_info->val); } else { + *temp++ = 0; + temp2 = temp; + temp = strstr(temp, ")"); *temp = 0; job->gid = atoi(temp2); job->gid_name = xstrdup(jobcomp_info->val); diff --git a/src/plugins/priority/multifactor/priority_multifactor.c b/src/plugins/priority/multifactor/priority_multifactor.c index 7dfab9b567b..72803acafd7 100644 --- a/src/plugins/priority/multifactor/priority_multifactor.c +++ b/src/plugins/priority/multifactor/priority_multifactor.c @@ -617,7 +617,7 @@ static void _get_priority_factors(time_t start_time, struct job_record *job_ptr) } if (weight_js) { - uint32_t cpu_cnt = 0; + uint32_t cpu_cnt = 0, min_nodes = 1; /* On the initial run of this we don't have total_cpus so go off the requesting. After the first shot total_cpus should be filled in. @@ -629,6 +629,8 @@ static void _get_priority_factors(time_t start_time, struct job_record *job_ptr) cpu_cnt = job_ptr->details->max_cpus; else if (job_ptr->details && job_ptr->details->min_cpus) cpu_cnt = job_ptr->details->min_cpus; + if (job_ptr->details) + min_nodes = job_ptr->details->min_nodes; if (flags & PRIORITY_FLAGS_SIZE_RELATIVE) { uint32_t time_limit = 1; @@ -656,8 +658,7 @@ static void _get_priority_factors(time_t start_time, struct job_record *job_ptr) } } else if (favor_small) { job_ptr->prio_factors->priority_js = - (double)(node_record_count - - job_ptr->details->min_nodes) + (double)(node_record_count - min_nodes) / (double)node_record_count; if (cpu_cnt) { job_ptr->prio_factors->priority_js += @@ -667,8 +668,7 @@ static void _get_priority_factors(time_t start_time, struct job_record *job_ptr) } } else { /* favor large */ job_ptr->prio_factors->priority_js = - (double)job_ptr->details->min_nodes - / (double)node_record_count; + (double)min_nodes / (double)node_record_count; if (cpu_cnt) { job_ptr->prio_factors->priority_js += (double)cpu_cnt / (double)cluster_cpus; @@ -691,7 +691,10 @@ static void _get_priority_factors(time_t start_time, struct job_record *job_ptr) qos_ptr->usage->norm_priority; } - job_ptr->prio_factors->nice = job_ptr->details->nice; + if (job_ptr->details) + job_ptr->prio_factors->nice = job_ptr->details->nice; + else + job_ptr->prio_factors->nice = NICE_OFFSET; } static uint32_t _get_priority_internal(time_t start_time, diff --git a/src/slurmctld/step_mgr.c b/src/slurmctld/step_mgr.c index 7ed392be783..c9e8e84e50e 100644 --- a/src/slurmctld/step_mgr.c +++ b/src/slurmctld/step_mgr.c @@ -1612,7 +1612,9 @@ static void _pick_step_cores(struct step_record *step_ptr, return; } } - if (use_all_cores) + /* The test for cores==0 is just to avoid CLANG errors. + * It should never happen */ + if (use_all_cores || (cores == 0)) return; /* We need to over-subscribe one or more cores. -- GitLab