example/nvme: Improve error check of input parsing by spdk_strtol

Change-Id: Ied40ba8d3e342f2374c7c8c8b46ac11e9502db4f
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/c/441630
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: wuzhouhui <wuzhouhui@kingsoft.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>

examples/nvme/arbitration/arbitration.c

@@ -689,50 +689,62 @@ parse_args(int argc, char **argv)
 	const char *workload_type	= NULL;
 	int op				= 0;
 	bool mix_specified		= false;
+	long int val;
 
 	while ((op = getopt(argc, argv, "c:l:i:m:q:s:t:w:M:a:b:n:h")) != -1) {
 		switch (op) {
 		case 'c':
 			g_arbitration.core_mask = optarg;
 			break;
-		case 'i':
-			g_arbitration.shm_id = atoi(optarg);
-			break;
-		case 'l':
-			g_arbitration.latency_tracking_enable = atoi(optarg);
-			break;
-		case 'm':
-			g_arbitration.max_completions = atoi(optarg);
-			break;
-		case 'q':
-			g_arbitration.queue_depth = atoi(optarg);
-			break;
-		case 's':
-			g_arbitration.io_size_bytes = atoi(optarg);
-			break;
-		case 't':
-			g_arbitration.time_in_sec = atoi(optarg);
-			break;
 		case 'w':
 			g_arbitration.workload_type = optarg;
 			break;
-		case 'M':
-			g_arbitration.rw_percentage = atoi(optarg);
-			mix_specified = true;
-			break;
-		case 'a':
-			g_arbitration.arbitration_mechanism = atoi(optarg);
-			break;
-		case 'b':
-			g_arbitration.arbitration_config = atoi(optarg);
-			break;
-		case 'n':
-			g_arbitration.io_count = atoi(optarg);
-			break;
 		case 'h':
-		default:
+		case '?':
 			usage(argv[0]);
 			return 1;
+		default:
+			val = spdk_strtol(optarg, 10);
+			if (val < 0) {
+				fprintf(stderr, "Converting a string to integer failed\n");
+				return val;
+			}
+			switch (op) {
+			case 'i':
+				g_arbitration.shm_id = val;
+				break;
+			case 'l':
+				g_arbitration.latency_tracking_enable = val;
+				break;
+			case 'm':
+				g_arbitration.max_completions = val;
+				break;
+			case 'q':
+				g_arbitration.queue_depth = val;
+				break;
+			case 's':
+				g_arbitration.io_size_bytes = val;
+				break;
+			case 't':
+				g_arbitration.time_in_sec = val;
+				break;
+			case 'M':
+				g_arbitration.rw_percentage = val;
+				mix_specified = true;
+				break;
+			case 'a':
+				g_arbitration.arbitration_mechanism = val;
+				break;
+			case 'b':
+				g_arbitration.arbitration_config = val;
+				break;
+			case 'n':
+				g_arbitration.io_count = val;
+				break;
+			default:
+				usage(argv[0]);
+				return -EINVAL;
+			}
 		}
 	}
 

examples/nvme/cmb_copy/cmb_copy.c

@@ -35,6 +35,7 @@
 
 #include "spdk/env.h"
 #include "spdk/nvme.h"
+#include "spdk/string.h"
 
 #define CMB_COPY_DELIM "-"
 #define CMB_COPY_READ 0
@@ -227,6 +228,7 @@ static void
 parse(char *in, struct nvme_io *io)
 {
 	char *tok = NULL;
+	long int val;
 
 	tok = strtok(in, CMB_COPY_DELIM);
 	if (tok == NULL) {
@@ -239,19 +241,31 @@ parse(char *in, struct nvme_io *io)
 	if (tok == NULL) {
 		goto err;
 	}
-	io->nsid  = atoi(tok);
+	val = spdk_strtol(tok, 10);
+	if (val < 0) {
+		goto err;
+	}
+	io->nsid  = (unsigned)val;
 
 	tok = strtok(NULL, CMB_COPY_DELIM);
 	if (tok == NULL) {
 		goto err;
 	}
-	io->slba  = atoi(tok);
+	val = spdk_strtol(tok, 10);
+	if (val < 0) {
+		goto err;
+	}
+	io->slba  = (unsigned)val;
 
 	tok = strtok(NULL, CMB_COPY_DELIM);
 	if (tok == NULL) {
 		goto err;
 	}
-	io->nlbas = atoi(tok);
+	val = spdk_strtol(tok, 10);
+	if (val < 0) {
+		goto err;
+	}
+	io->nlbas = (unsigned)val;
 
 	tok = strtok(NULL, CMB_COPY_DELIM);
 	if (tok != NULL) {

examples/nvme/fio_plugin/fio_plugin.c

@@ -240,15 +240,17 @@ attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid,
 	struct fio_file		*f = fio_thread->current_f;
 	uint32_t		ns_id;
 	char			*p;
+	long int		tmp;
 
 	p = strstr(f->file_name, "ns=");
 	assert(p != NULL);
-	ns_id = atoi(p + 3);
+	tmp = spdk_strtol(p + 3, 10);
-	if (!ns_id) {
+	if (tmp <= 0) {
-		SPDK_ERRLOG("namespace id should be >=1, but current value=0\n");
+		SPDK_ERRLOG("namespace id should be >=1, but was invalid: %ld\n", tmp);
 		g_error = true;
 		return;
 	}
+	ns_id = (uint32_t)tmp;
 
 	fio_ctrlr = get_fio_ctrlr(trid);
 	/* it is a new ctrlr and needs to be added */

examples/nvme/hotplug/hotplug.c

@@ -35,6 +35,7 @@
 
 #include "spdk/nvme.h"
 #include "spdk/queue.h"
+#include "spdk/string.h"
 
 struct dev_ctx {
 	TAILQ_ENTRY(dev_ctx)	tailq;
@@ -397,23 +398,34 @@ static int
 parse_args(int argc, char **argv)
 {
 	int op;
+	long int val;
 
 	/* default value */
 	g_time_in_sec = 0;
 
 	while ((op = getopt(argc, argv, "i:n:r:t:")) != -1) {
+		if (op == '?') {
+			usage(argv[0]);
+			return 1;
+		}
+
+		val = spdk_strtol(optarg, 10);
+		if (val < 0) {
+			fprintf(stderr, "Converting a string to integer failed\n");
+			return val;
+		}
 		switch (op) {
 		case 'i':
-			g_shm_id = atoi(optarg);
+			g_shm_id = val;
 			break;
 		case 'n':
-			g_expected_insert_times = atoi(optarg);
+			g_expected_insert_times = val;
 			break;
 		case 'r':
-			g_expected_removal_times = atoi(optarg);
+			g_expected_removal_times = val;
 			break;
 		case 't':
-			g_time_in_sec = atoi(optarg);
+			g_time_in_sec = val;
 			break;
 		default:
 			usage(argv[0]);

examples/nvme/identify/identify.c

@@ -1626,16 +1626,24 @@ parse_args(int argc, char **argv)
 	while ((op = getopt(argc, argv, "d:i:p:r:xHL:")) != -1) {
 		switch (op) {
 		case 'd':
-			g_dpdk_mem = atoi(optarg);
+			g_dpdk_mem = spdk_strtol(optarg, 10);
+			if (g_dpdk_mem < 0) {
+				fprintf(stderr, "Invalid DPDK memory size\n");
+				return g_dpdk_mem;
+			}
 			break;
 		case 'i':
-			g_shm_id = atoi(optarg);
+			g_shm_id = spdk_strtol(optarg, 10);
+			if (g_shm_id < 0) {
+				fprintf(stderr, "Invalid shared memory ID\n");
+				return g_shm_id;
+			}
 			break;
 		case 'p':
-			g_master_core = atoi(optarg);
+			g_master_core = spdk_strtol(optarg, 10);
 			if (g_master_core < 0) {
 				fprintf(stderr, "Invalid core number\n");
-				return 1;
+				return g_master_core;
 			}
 			snprintf(g_core_mask, sizeof(g_core_mask), "0x%llx", 1ULL << g_master_core);
 			break;

examples/nvme/nvme_manage/nvme_manage.c

@@ -35,6 +35,7 @@
 
 #include "spdk/nvme.h"
 #include "spdk/env.h"
+#include "spdk/string.h"
 #include "spdk/util.h"
 
 #define MAX_DEVS 64
@@ -870,7 +871,11 @@ parse_args(int argc, char **argv)
 	while ((op = getopt(argc, argv, "i:")) != -1) {
 		switch (op) {
 		case 'i':
-			g_shm_id = atoi(optarg);
+			g_shm_id = spdk_strtol(optarg, 10);
+			if (g_shm_id < 0) {
+				fprintf(stderr, "Invalid shared memory ID\n");
+				return g_shm_id;
+			}
 			break;
 		default:
 			args_usage(argv[0]);

examples/nvme/perf/perf.c

@@ -1301,7 +1301,7 @@ add_trid(const char *trid_str)
 		memcpy(nsid_str, ns, len);
 		nsid_str[len] = '\0';
 
-		nsid = atoi(nsid_str);
+		nsid = spdk_strtol(nsid_str, 10);
 		if (nsid <= 0 || nsid > 65535) {
 			fprintf(stderr, "NVMe namespace IDs must be less than 65536 and greater than 0\n");
 			free(trid_entry);
@@ -1411,6 +1411,7 @@ parse_args(int argc, char **argv)
 	const char *workload_type;
 	int op;
 	bool mix_specified = false;
+	long int val;
 
 	/* default value */
 	g_queue_depth = 0;
@@ -1423,6 +1424,43 @@ parse_args(int argc, char **argv)
 
 	while ((op = getopt(argc, argv, "c:e:i:lm:o:q:r:s:t:w:DHILM:")) != -1) {
 		switch (op) {
+		case 'i':
+		case 'm':
+		case 'o':
+		case 'q':
+		case 's':
+		case 't':
+		case 'M':
+			val = spdk_strtol(optarg, 10);
+			if (val < 0) {
+				fprintf(stderr, "Converting a string to integer failed\n");
+				return val;
+			}
+			switch (op) {
+			case 'i':
+				g_shm_id = val;
+				break;
+			case 'm':
+				g_max_completions = val;
+				break;
+			case 'o':
+				g_io_size_bytes = val;
+				break;
+			case 'q':
+				g_queue_depth = val;
+				break;
+			case 's':
+				g_dpdk_mem = val;
+				break;
+			case 't':
+				g_time_in_sec = val;
+				break;
+			case 'M':
+				g_rw_percentage = val;
+				mix_specified = true;
+				break;
+			}
+			break;
 		case 'c':
 			g_core_mask = optarg;
 			break;
@@ -1432,33 +1470,15 @@ parse_args(int argc, char **argv)
 				return 1;
 			}
 			break;
-		case 'i':
-			g_shm_id = atoi(optarg);
-			break;
 		case 'l':
 			g_latency_ssd_tracking_enable = true;
 			break;
-		case 'm':
-			g_max_completions = atoi(optarg);
-			break;
-		case 'o':
-			g_io_size_bytes = atoi(optarg);
-			break;
-		case 'q':
-			g_queue_depth = atoi(optarg);
-			break;
 		case 'r':
 			if (add_trid(optarg)) {
 				usage(argv[0]);
 				return 1;
 			}
 			break;
-		case 's':
-			g_dpdk_mem = atoi(optarg);
-			break;
-		case 't':
-			g_time_in_sec = atoi(optarg);
-			break;
 		case 'w':
 			workload_type = optarg;
 			break;
@@ -1474,10 +1494,6 @@ parse_args(int argc, char **argv)
 		case 'L':
 			g_latency_sw_tracking_level++;
 			break;
-		case 'M':
-			g_rw_percentage = atoi(optarg);
-			mix_specified = true;
-			break;
 		default:
 			usage(argv[0]);
 			return 1;