plugins/solidigm: Fix parse-telemetry-log command parsing error handling.

Returning errno as negative number as rest of code.
Turned -source-file into a regular file argument.
Replaced goto with clean_up variable attributes.

Signed-off-by: Leonardo da Cunha <[email protected]>

Author: lgdacunh

plugins/solidigm/solidigm-nvme.h

@@ -13,7 +13,7 @@
 
 #include "cmd.h"
 
-#define SOLIDIGM_PLUGIN_VERSION "1.9"
+#define SOLIDIGM_PLUGIN_VERSION "1.10"
 
 PLUGIN(NAME("solidigm", "Solidigm vendor specific extensions", SOLIDIGM_PLUGIN_VERSION),
 	COMMAND_LIST(

plugins/solidigm/solidigm-telemetry.c

@@ -29,7 +29,7 @@ static int read_file2buffer(char *file_name, char **buffer, size_t *length)
 	FILE *fd = fopen(file_name, "rb");
 
 	if (!fd)
-		return errno;
+		return -errno;
 
 	fseek(fd, 0, SEEK_END);
 	size_t length_bytes = ftell(fd);
@@ -39,7 +39,7 @@ static int read_file2buffer(char *file_name, char **buffer, size_t *length)
 	*buffer = malloc(length_bytes);
 	if (!*buffer) {
 		fclose(fd);
-		return errno;
+		return -errno;
 	}
 	*length = fread(*buffer, 1, length_bytes, fd);
 	fclose(fd);
@@ -51,99 +51,117 @@ struct config {
 	bool ctrl_init;
 	int  data_area;
 	char *cfg_file;
-	bool is_input_file;
+	char *binary_file;
 };
 
+static void cleanup_json_object(struct json_object **jobj_ptr)
+{
+	json_free_object(*jobj_ptr);
+	*jobj_ptr = NULL;
+}
+
 int solidigm_get_telemetry_log(int argc, char **argv, struct command *cmd, struct plugin *plugin)
 {
 	const char *desc = "Parse Solidigm Telemetry log";
 	const char *hgen = "Controls when to generate new host initiated report. Default value '1' generates new host initiated report, value '0' causes retrieval of existing log.";
 	const char *cgen = "Gather report generated by the controller.";
 	const char *dgen = "Pick which telemetry data area to report. Default is 3 to fetch areas 1-3. Valid options are 1, 2, 3, 4.";
 	const char *cfile = "JSON configuration file";
-	const char *sfile = "data source <device> is binary file containing log dump instead of block or character device";
-	struct nvme_dev *dev;
+	const char *sfile = "binary file containing log dump";
+	bool has_binary_file = false;
+
+	_cleanup_nvme_dev_ struct nvme_dev *dev = NULL;
+
+	_cleanup_free_ struct nvme_telemetry_log *tlog = NULL;
+
+	__attribute__((cleanup(cleanup_json_object))) struct json_object *configuration = NULL;
+
+	__attribute__((cleanup(cleanup_json_object))) struct json_object *root =
+		json_create_object();
 
 	struct telemetry_log tl = {
-		.root = json_create_object(),
-		.log = NULL,
+		.root = root,
 	};
 
 	struct config cfg = {
 		.host_gen   = 1,
 		.ctrl_init  = false,
-		.data_area  = -1,
-		.cfg_file   = NULL,
-		.is_input_file = false,
 	};
 
 	OPT_ARGS(opts) = {
 		OPT_UINT("host-generate",   'g', &cfg.host_gen,  hgen),
 		OPT_FLAG("controller-init", 'c', &cfg.ctrl_init, cgen),
 		OPT_UINT("data-area",       'd', &cfg.data_area, dgen),
 		OPT_FILE("config-file",     'j', &cfg.cfg_file, cfile),
-		OPT_FLAG("source-file",     's', &cfg.is_input_file, sfile),
-		OPT_INCR("verbose",      'v', &nvme_cfg.verbose, verbose),
+		OPT_FILE("source-file",     's', &cfg.binary_file, sfile),
+		OPT_INCR("verbose",         'v', &nvme_cfg.verbose, verbose),
 		OPT_END()
 	};
 
 	int err = argconfig_parse(argc, argv, desc, opts);
 
-	if (err)
-		goto ret;
+	if (err) {
+		nvme_show_status(err);
+		return err;
+	}
 
 	/* When not selected on the command line, get minimum data area required */
-	if (cfg.data_area == -1)
-		cfg.data_area =  cfg.cfg_file ? 3 : 1;
-
-	if (cfg.is_input_file) {
-		if (optind >= argc) {
-			err = errno = EINVAL;
-			perror(argv[0]);
-			goto ret;
+	if (!argconfig_parse_seen(opts, "data-area"))
+		cfg.data_area = argconfig_parse_seen(opts, "config-file") ? 3 : 1;
+
+	has_binary_file = argconfig_parse_seen(opts, "source-file");
+	if (has_binary_file) {
+		// If a binary file is provided, we don't want to open a device.
+		// GNU getopt() permutes the contents of argv as it scans,
+		// so that eventually all the nonoptions are at the end.
+		if (argc > optind) {
+			errno = EINVAL;
+			err = -errno;
+			nvme_show_status(err);
+			return err;
 		}
-		char *binary_file_name = argv[optind];
-
-		err = read_file2buffer(binary_file_name, (char **)&tl.log, &tl.log_size);
+		err = read_file2buffer(cfg.binary_file, (char **)&tlog, &tl.log_size);
 	} else {
 		err = parse_and_open(&dev, argc, argv, desc, opts);
 	}
-	if (err)
-		goto ret;
+	if (err) {
+		nvme_show_status(err);
+		return err;
+	}
 
 	if (cfg.host_gen > 1) {
 		SOLIDIGM_LOG_WARNING("Invalid host-generate value '%d'", cfg.host_gen);
-		err = EINVAL;
-		goto close_fd;
+		err = -EINVAL;
+		nvme_show_status(err);
+		return err;
 	}
 
-	if (cfg.cfg_file) {
-		char *conf_str = NULL;
+	if (argconfig_parse_seen(opts, "config-file")) {
+		_cleanup_free_ char *conf_str = NULL;
 		size_t length = 0;
 
 		err = read_file2buffer(cfg.cfg_file, &conf_str, &length);
 		if (err) {
-			SOLIDIGM_LOG_WARNING("Failed to open JSON configuration file %s: %s!",
-				cfg.cfg_file, strerror(err));
-			goto close_fd;
+			nvme_show_status(err);
+			return err;
 		}
 		struct json_tokener *jstok = json_tokener_new();
 
-		tl.configuration = json_tokener_parse_ex(jstok, conf_str, length);
-		free(conf_str);
+		configuration = json_tokener_parse_ex(jstok, conf_str, length);
 		if (jstok->err != json_tokener_success)	{
 			SOLIDIGM_LOG_WARNING("Parsing error on JSON configuration file %s: %s (at offset %d)",
 					     cfg.cfg_file,
 					     json_tokener_error_desc(jstok->err),
 					     jstok->char_offset);
 			json_tokener_free(jstok);
 			err = EINVAL;
-			goto close_fd;
+			return err;
 		}
 		json_tokener_free(jstok);
+		tl.configuration = configuration;
 	}
 
-	if (!cfg.is_input_file) {
+	if (!has_binary_file) {
 		size_t max_data_tx;
 		size_t power2;
 		__u8 mdts = 0;
@@ -152,11 +170,11 @@ int solidigm_get_telemetry_log(int argc, char **argv, struct command *cmd, struc
 		if (err < 0) {
 			SOLIDIGM_LOG_WARNING("identify_ctrl: %s",
 					     nvme_strerror(errno));
-			goto close_fd;
+			return err;
 		} else if (err > 0) {
 			nvme_show_status(err);
 			SOLIDIGM_LOG_WARNING("Failed to acquire identify ctrl %d!", err);
-			goto close_fd;
+			return err;
 		}
 		power2 = max_data_tx / NVME_LOG_PAGE_PDU_SIZE;
 		while (power2 && !(1 & power2)) {
@@ -165,31 +183,22 @@ int solidigm_get_telemetry_log(int argc, char **argv, struct command *cmd, struc
 		}
 
 		err = sldgm_dynamic_telemetry(dev_fd(dev), cfg.host_gen, cfg.ctrl_init, true,
-					      mdts, cfg.data_area, &tl.log, &tl.log_size);
+					      mdts, cfg.data_area, &tlog, &tl.log_size);
 		if (err < 0) {
 			SOLIDIGM_LOG_WARNING("get-telemetry-log: %s",
 					     nvme_strerror(errno));
-			goto close_fd;
+			return err;
 		} else if (err > 0) {
 			nvme_show_status(err);
 			SOLIDIGM_LOG_WARNING("Failed to acquire telemetry log %d!", err);
-			goto close_fd;
+			return err;
 		}
 	}
+	tl.log = tlog;
 	solidigm_telemetry_log_data_areas_parse(&tl, cfg.data_area);
 
 	json_print_object(tl.root, NULL);
-	json_free_object(tl.root);
 	printf("\n");
 
-close_fd:
-	if (!cfg.is_input_file) {
-		/* Redundant close() to make static code analysis happy */
-		close(dev->direct.fd);
-		dev_close(dev);
-	}
-ret:
-	json_free_object(tl.configuration);
-	free(tl.log);
 	return err;
 }