From fee33a291a991e799b6de25b655eceb637f09c82 Mon Sep 17 00:00:00 2001
From: Tim Wickberg <tim@schedmd.com>
Date: Fri, 13 Jan 2017 02:00:58 -0500
Subject: [PATCH] Replace all usage of pack_bit_fmt with pack_bit_str_hex.

Bug 3257.
---
 src/common/slurm_protocol_pack.c | 80 ++++++++++----------------------
 src/slurmctld/job_mgr.c          |  8 ++--
 src/slurmctld/partition_mgr.c    |  2 +-
 src/slurmctld/reservation.c      |  2 +-
 src/slurmctld/step_mgr.c         | 41 ++++++++--------
 5 files changed, 52 insertions(+), 81 deletions(-)

diff --git a/src/common/slurm_protocol_pack.c b/src/common/slurm_protocol_pack.c
index c253228c61a..04639181c8f 100644
--- a/src/common/slurm_protocol_pack.c
+++ b/src/common/slurm_protocol_pack.c
@@ -5396,7 +5396,6 @@ _unpack_partition_info_members(partition_info_t * part, Buf buffer,
 			       uint16_t protocol_version)
 {
 	uint32_t uint32_tmp;
-	char *node_inx_str = NULL;
 
 	if (protocol_version >= SLURM_17_02_PROTOCOL_VERSION) {
 		safe_unpackstr_xmalloc(&part->name, &uint32_tmp, buffer);
@@ -5437,19 +5436,15 @@ _unpack_partition_info_members(partition_info_t * part, Buf buffer,
 		safe_unpackstr_xmalloc(&part->deny_qos, &uint32_tmp,
 				       buffer);
 		safe_unpackstr_xmalloc(&part->nodes, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&node_inx_str, &uint32_tmp, buffer);
-		if (node_inx_str == NULL)
-			part->node_inx = bitfmt2int("");
-		else {
-			part->node_inx = bitfmt2int(node_inx_str);
-			xfree(node_inx_str);
-			node_inx_str = NULL;
-		}
+
+		unpack_bit_str_hex_as_inx(&part->node_inx, buffer);
+
 		safe_unpackstr_xmalloc(&part->billing_weights_str, &uint32_tmp,
 				       buffer);
 		safe_unpackstr_xmalloc(&part->tres_fmt_str, &uint32_tmp,
 				       buffer);
 	} else if (protocol_version >= SLURM_16_05_PROTOCOL_VERSION) {
+		char *node_inx_str = NULL;
 		uint32_t tmp_mem;
 		safe_unpackstr_xmalloc(&part->name, &uint32_tmp, buffer);
 		if (part->name == NULL)
@@ -5503,6 +5498,7 @@ _unpack_partition_info_members(partition_info_t * part, Buf buffer,
 		safe_unpackstr_xmalloc(&part->tres_fmt_str, &uint32_tmp,
 				       buffer);
 	} else if (protocol_version >= SLURM_MIN_PROTOCOL_VERSION) {
+		char *node_inx_str = NULL;
 		uint32_t tmp_mem;
 		safe_unpackstr_xmalloc(&part->name, &uint32_tmp, buffer);
 		if (part->name == NULL)
@@ -5653,7 +5649,6 @@ static int
 _unpack_reserve_info_members(reserve_info_t * resv, Buf buffer,
 			     uint16_t protocol_version)
 {
-	char *node_inx_str = NULL;
 	uint32_t uint32_tmp;
 
 	if (protocol_version >= SLURM_17_02_PROTOCOL_VERSION) {
@@ -5673,15 +5668,11 @@ _unpack_reserve_info_members(reserve_info_t * resv, Buf buffer,
 
 		safe_unpackstr_xmalloc(&resv->tres_str, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&resv->users,	&uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&node_inx_str,   &uint32_tmp, buffer);
-		if (node_inx_str == NULL)
-			resv->node_inx = bitfmt2int("");
-		else {
-			resv->node_inx = bitfmt2int(node_inx_str);
-			xfree(node_inx_str);
-			node_inx_str = NULL;
-		}
+
+		unpack_bit_str_hex_as_inx(&resv->node_inx, buffer);
 	} else if (protocol_version >= SLURM_MIN_PROTOCOL_VERSION) {
+		char *node_inx_str = NULL;
+
 		safe_unpackstr_xmalloc(&resv->accounts,	&uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&resv->burst_buffer,&uint32_tmp, buffer);
 		safe_unpack32(&resv->core_cnt,          buffer);
@@ -5714,7 +5705,6 @@ _unpack_reserve_info_members(reserve_info_t * resv, Buf buffer,
 	return SLURM_SUCCESS;
 
 unpack_error:
-	xfree(node_inx_str);
 	slurm_free_reserve_info_members(resv);
 	return SLURM_ERROR;
 }
@@ -5730,7 +5720,6 @@ _unpack_job_step_info_members(job_step_info_t * step, Buf buffer,
 			      uint16_t protocol_version)
 {
 	uint32_t uint32_tmp = 0;
-	char *node_inx_str = NULL;
 
 	if (protocol_version >= SLURM_17_02_PROTOCOL_VERSION) {
 		safe_unpack32(&step->array_job_id, buffer);
@@ -5758,15 +5747,9 @@ _unpack_job_step_info_members(job_step_info_t * step, Buf buffer,
 		safe_unpackstr_xmalloc(&step->nodes, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&step->name, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&step->network, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&node_inx_str, &uint32_tmp, buffer);
+		unpack_bit_str_hex_as_inx(&step->node_inx, buffer);
 		safe_unpackstr_xmalloc(&step->ckpt_dir, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&step->gres, &uint32_tmp, buffer);
-		if (node_inx_str == NULL)
-			step->node_inx = bitfmt2int("");
-		else {
-			step->node_inx = bitfmt2int(node_inx_str);
-			xfree(node_inx_str);
-		}
 
 		if (select_g_select_jobinfo_unpack(&step->select_jobinfo,
 						   buffer, protocol_version))
@@ -5778,6 +5761,7 @@ _unpack_job_step_info_members(job_step_info_t * step, Buf buffer,
 		safe_unpack32(&step->packjobid, buffer);
 		safe_unpack32(&step->packstepid, buffer);
 	} else if (protocol_version >= SLURM_16_05_PROTOCOL_VERSION) {
+		char *node_inx_str = NULL;
 		safe_unpack32(&step->array_job_id, buffer);
 		safe_unpack32(&step->array_task_id, buffer);
 		safe_unpack32(&step->job_id, buffer);
@@ -5802,14 +5786,14 @@ _unpack_job_step_info_members(job_step_info_t * step, Buf buffer,
 		safe_unpackstr_xmalloc(&step->name, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&step->network, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&node_inx_str, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&step->ckpt_dir, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&step->gres, &uint32_tmp, buffer);
 		if (node_inx_str == NULL)
 			step->node_inx = bitfmt2int("");
 		else {
 			step->node_inx = bitfmt2int(node_inx_str);
 			xfree(node_inx_str);
 		}
+		safe_unpackstr_xmalloc(&step->ckpt_dir, &uint32_tmp, buffer);
+		safe_unpackstr_xmalloc(&step->gres, &uint32_tmp, buffer);
 
 		if (select_g_select_jobinfo_unpack(&step->select_jobinfo,
 						   buffer, protocol_version))
@@ -5819,6 +5803,7 @@ _unpack_job_step_info_members(job_step_info_t * step, Buf buffer,
 				       &uint32_tmp, buffer);
 		safe_unpack16(&step->start_protocol_ver, buffer);
 	} else if (protocol_version >= SLURM_MIN_PROTOCOL_VERSION) {
+		char *node_inx_str = NULL;
 		safe_unpack32(&step->array_job_id, buffer);
 		safe_unpack32(&step->array_task_id, buffer);
 		safe_unpack32(&step->job_id, buffer);
@@ -5843,14 +5828,14 @@ _unpack_job_step_info_members(job_step_info_t * step, Buf buffer,
 		safe_unpackstr_xmalloc(&step->name, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&step->network, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&node_inx_str, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&step->ckpt_dir, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&step->gres, &uint32_tmp, buffer);
 		if (node_inx_str == NULL)
 			step->node_inx = bitfmt2int("");
 		else {
 			step->node_inx = bitfmt2int(node_inx_str);
 			xfree(node_inx_str);
 		}
+		safe_unpackstr_xmalloc(&step->ckpt_dir, &uint32_tmp, buffer);
+		safe_unpackstr_xmalloc(&step->gres, &uint32_tmp, buffer);
 
 		if (select_g_select_jobinfo_unpack(&step->select_jobinfo,
 						   buffer, protocol_version))
@@ -5871,7 +5856,6 @@ unpack_error:
 	   since this is freed in _unpack_job_step_info_response_msg
 	*/
 	//slurm_free_job_step_info_members(step);
-	xfree(node_inx_str);
 	return SLURM_ERROR;
 }
 
@@ -6053,7 +6037,6 @@ _unpack_job_info_members(job_info_t * job, Buf buffer,
 {
 	uint32_t uint32_tmp = 0;
 	uint8_t uint8_tmp = 0;
-	char *node_inx_str;
 	multi_core_data_t *mc_ptr;
 
 	job->ntasks_per_node = (uint16_t)NO_VAL;
@@ -6133,13 +6116,8 @@ _unpack_job_info_members(job_info_t * job, Buf buffer,
 		safe_unpack32(&job->wait4switch, buffer);
 
 		safe_unpackstr_xmalloc(&job->alloc_node, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&node_inx_str, &uint32_tmp, buffer);
-		if (node_inx_str == NULL)
-			job->node_inx = bitfmt2int("");
-		else {
-			job->node_inx = bitfmt2int(node_inx_str);
-			xfree(node_inx_str);
-		}
+
+		unpack_bit_str_hex_as_inx(&job->node_inx, buffer);
 
 		if (select_g_select_jobinfo_unpack(&job->select_jobinfo,
 						   buffer, protocol_version))
@@ -6172,23 +6150,13 @@ _unpack_job_info_members(job_info_t * job, Buf buffer,
 
 		safe_unpack64(&job->pn_min_memory, buffer);
 		safe_unpack32(&job->pn_min_tmp_disk, buffer);
-
 		safe_unpackstr_xmalloc(&job->req_nodes, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&node_inx_str, &uint32_tmp, buffer);
-		if (node_inx_str == NULL)
-			job->req_node_inx = bitfmt2int("");
-		else {
-			job->req_node_inx = bitfmt2int(node_inx_str);
-			xfree(node_inx_str);
-		}
+
+		unpack_bit_str_hex_as_inx(&job->req_node_inx, buffer);
+
 		safe_unpackstr_xmalloc(&job->exc_nodes, &uint32_tmp, buffer);
-		safe_unpackstr_xmalloc(&node_inx_str, &uint32_tmp, buffer);
-		if (node_inx_str == NULL)
-			job->exc_node_inx = bitfmt2int("");
-		else {
-			job->exc_node_inx = bitfmt2int(node_inx_str);
-			xfree(node_inx_str);
-		}
+
+		unpack_bit_str_hex_as_inx(&job->exc_node_inx, buffer);
 
 		safe_unpackstr_xmalloc(&job->std_err, &uint32_tmp, buffer);
 		safe_unpackstr_xmalloc(&job->std_in,  &uint32_tmp, buffer);
@@ -6220,6 +6188,7 @@ _unpack_job_info_members(job_info_t * job, Buf buffer,
 		safe_unpackstr_xmalloc(&job->fed_siblings_str, &uint32_tmp,
 				       buffer);
 	} else if (protocol_version >= SLURM_16_05_PROTOCOL_VERSION) {
+		char *node_inx_str;
 		uint32_t tmp_mem;
 		safe_unpack32(&job->array_job_id, buffer);
 		safe_unpack32(&job->array_task_id, buffer);
@@ -6371,6 +6340,7 @@ _unpack_job_info_members(job_info_t * job, Buf buffer,
 				       &uint32_tmp, buffer);
 		safe_unpack16(&job->start_protocol_ver, buffer);
 	} else if (protocol_version >= SLURM_MIN_PROTOCOL_VERSION) {
+		char *node_inx_str;
 		uint32_t tmp_mem;
 		uint16_t old_nice = 0;
 		safe_unpack32(&job->array_job_id, buffer);
diff --git a/src/slurmctld/job_mgr.c b/src/slurmctld/job_mgr.c
index 31ddf71a24d..58404e160f1 100644
--- a/src/slurmctld/job_mgr.c
+++ b/src/slurmctld/job_mgr.c
@@ -8756,9 +8756,9 @@ void pack_job(struct job_record *dump_job_ptr, uint16_t show_flags, Buf buffer,
 
 		packstr(dump_job_ptr->alloc_node, buffer);
 		if (!IS_JOB_COMPLETING(dump_job_ptr))
-			pack_bit_fmt(dump_job_ptr->node_bitmap, buffer);
+			pack_bit_str_hex(dump_job_ptr->node_bitmap, buffer);
 		else
-			pack_bit_fmt(dump_job_ptr->node_bitmap_cg, buffer);
+			pack_bit_str_hex(dump_job_ptr->node_bitmap_cg, buffer);
 
 		select_g_select_jobinfo_pack(dump_job_ptr->select_jobinfo,
 					     buffer, protocol_version);
@@ -9505,9 +9505,9 @@ static void _pack_pending_job_details(struct job_details *detail_ptr,
 			pack32(detail_ptr->pn_min_tmp_disk, buffer);
 
 			packstr(detail_ptr->req_nodes, buffer);
-			pack_bit_fmt(detail_ptr->req_node_bitmap, buffer);
+			pack_bit_str_hex(detail_ptr->req_node_bitmap, buffer);
 			packstr(detail_ptr->exc_nodes, buffer);
-			pack_bit_fmt(detail_ptr->exc_node_bitmap, buffer);
+			pack_bit_str_hex(detail_ptr->exc_node_bitmap, buffer);
 
 			packstr(detail_ptr->std_err, buffer);
 			packstr(detail_ptr->std_in, buffer);
diff --git a/src/slurmctld/partition_mgr.c b/src/slurmctld/partition_mgr.c
index b6fa6edb693..e5a957acc95 100644
--- a/src/slurmctld/partition_mgr.c
+++ b/src/slurmctld/partition_mgr.c
@@ -1286,7 +1286,7 @@ void pack_part(struct part_record *part_ptr, Buf buffer,
 		packstr(part_ptr->deny_accounts, buffer);
 		packstr(part_ptr->deny_qos, buffer);
 		packstr(part_ptr->nodes, buffer);
-		pack_bit_fmt(part_ptr->node_bitmap, buffer);
+		pack_bit_str_hex(part_ptr->node_bitmap, buffer);
 		packstr(part_ptr->billing_weights_str, buffer);
 		packstr(part_ptr->tres_fmt_str, buffer);
 	} else if (protocol_version >= SLURM_16_05_PROTOCOL_VERSION) {
diff --git a/src/slurmctld/reservation.c b/src/slurmctld/reservation.c
index eb9eebc6cbe..91e3a31320a 100644
--- a/src/slurmctld/reservation.c
+++ b/src/slurmctld/reservation.c
@@ -1538,7 +1538,7 @@ static void _pack_resv(slurmctld_resv_t *resv_ptr, Buf buffer,
 			packstr(resv_ptr->tres_str,	buffer);
 			pack8(resv_ptr->user_not,	buffer);
 		} else {
-			pack_bit_fmt(resv_ptr->node_bitmap, buffer);
+			pack_bit_str_hex(resv_ptr->node_bitmap, buffer);
 		}
 	} else if (protocol_version >= SLURM_16_05_PROTOCOL_VERSION) {
 		packstr(resv_ptr->accounts,	buffer);
diff --git a/src/slurmctld/step_mgr.c b/src/slurmctld/step_mgr.c
index 7c947df4d2c..bf51829f7d7 100644
--- a/src/slurmctld/step_mgr.c
+++ b/src/slurmctld/step_mgr.c
@@ -2956,7 +2956,7 @@ static void _pack_ctld_job_step_info(struct step_record *step_ptr, Buf buffer,
 		packstr(node_list, buffer);
 		packstr(step_ptr->name, buffer);
 		packstr(step_ptr->network, buffer);
-		pack_bit_fmt(pack_bitstr, buffer);
+		pack_bit_str_hex(pack_bitstr, buffer);
 		packstr(step_ptr->ckpt_dir, buffer);
 		packstr(step_ptr->gres, buffer);
 		select_g_select_jobinfo_pack(step_ptr->select_jobinfo, buffer,
@@ -3792,18 +3792,9 @@ extern int dump_job_step_state(void *x, void *arg)
 	pack64(step_ptr->pn_min_memory, buffer);
 	pack32(step_ptr->exit_code, buffer);
 	if (step_ptr->exit_code != NO_VAL) {
-		uint16_t bit_cnt = 0;
-		if (step_ptr->exit_node_bitmap)
-			bit_cnt = bit_size(step_ptr->exit_node_bitmap);
-		pack_bit_fmt(step_ptr->exit_node_bitmap, buffer);
-		pack16(bit_cnt, buffer);
+		pack_bit_str_hex(step_ptr->exit_node_bitmap, buffer);
 	}
-	if (step_ptr->core_bitmap_job) {
-		uint32_t core_size = bit_size(step_ptr->core_bitmap_job);
-		pack32(core_size, buffer);
-		pack_bit_fmt(step_ptr->core_bitmap_job, buffer);
-	} else
-		pack32((uint32_t) 0, buffer);
+	pack_bit_str_hex(step_ptr->core_bitmap_job, buffer);
 	pack32(step_ptr->time_limit, buffer);
 	pack32(step_ptr->cpu_freq_min, buffer);
 	pack32(step_ptr->cpu_freq_max, buffer);
@@ -3853,11 +3844,12 @@ extern int load_step_state(struct job_record *job_ptr, Buf buffer,
 			   uint16_t protocol_version)
 {
 	struct step_record *step_ptr = NULL;
+	bitstr_t *exit_node_bitmap = NULL, *core_bitmap_job = NULL;
 	uint8_t no_kill;
 	uint16_t cyclic_alloc, port, batch_step, bit_cnt;
 	uint16_t start_protocol_ver = SLURM_MIN_PROTOCOL_VERSION;
 	uint16_t ckpt_interval, cpus_per_task, resv_port_cnt, state;
-	uint32_t core_size, cpu_count, exit_code, name_len, srun_pid = 0;
+	uint32_t core_size = 0, cpu_count, exit_code, name_len, srun_pid = 0;
 	uint32_t step_id, time_limit, cpu_freq_min, cpu_freq_max, cpu_freq_gov;
 	uint64_t pn_min_memory;
 	time_t start_time, pre_sus_time, tot_sus_time, ckpt_time;
@@ -3888,12 +3880,10 @@ extern int load_step_state(struct job_record *job_ptr, Buf buffer,
 		safe_unpack64(&pn_min_memory, buffer);
 		safe_unpack32(&exit_code, buffer);
 		if (exit_code != NO_VAL) {
-			safe_unpackstr_xmalloc(&bit_fmt, &name_len, buffer);
-			safe_unpack16(&bit_cnt, buffer);
+			unpack_bit_str_hex(&exit_node_bitmap, buffer);
 		}
-		safe_unpack32(&core_size, buffer);
-		if (core_size)
-			safe_unpackstr_xmalloc(&core_job, &name_len, buffer);
+		unpack_bit_str_hex(&core_bitmap_job, buffer);
+
 		safe_unpack32(&time_limit, buffer);
 		safe_unpack32(&cpu_freq_min, buffer);
 		safe_unpack32(&cpu_freq_max, buffer);
@@ -4092,7 +4082,12 @@ extern int load_step_state(struct job_record *job_ptr, Buf buffer,
 		step_ptr->ext_sensors = ext_sensors_alloc();
 
 	step_ptr->exit_code    = exit_code;
-	if (bit_fmt) {
+
+	if (exit_node_bitmap) {
+		step_ptr->exit_node_bitmap = exit_node_bitmap;
+		exit_node_bitmap = NULL;
+	} else if (bit_fmt) {
+		/* pre-17.02 compatibility */
 		/* NOTE: This is only recovered if a job step completion
 		 * is actively in progress at step save time. Otherwise
 		 * the bitmap is NULL. */
@@ -4103,7 +4098,11 @@ extern int load_step_state(struct job_record *job_ptr, Buf buffer,
 		}
 		xfree(bit_fmt);
 	}
-	if (core_size) {
+	if (core_bitmap_job) {
+		step_ptr->core_bitmap_job = core_bitmap_job;
+		core_bitmap_job = NULL;
+	} else if (core_size) {
+		/* pre-17.02 compatibility */
 		step_ptr->core_bitmap_job = bit_alloc(core_size);
 		if (bit_unfmt(step_ptr->core_bitmap_job, core_job)) {
 			error("error recovering core_bitmap_job from %s",
@@ -4127,6 +4126,8 @@ unpack_error:
 	xfree(ckpt_dir);
 	xfree(gres);
 	FREE_NULL_LIST(gres_list);
+	bit_free(exit_node_bitmap);
+	bit_free(core_bitmap_job);
 	xfree(bit_fmt);
 	xfree(core_job);
 	if (switch_tmp)
-- 
GitLab