From a3e9352907e40e6a70e517e7f82c57fc8045691c Mon Sep 17 00:00:00 2001 From: Leonardo da Cunha Date: Wed, 19 Mar 2025 18:03:52 -0700 Subject: [PATCH] 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 --- plugins/solidigm/solidigm-nvme.h | 2 +- plugins/solidigm/solidigm-telemetry.c | 115 ++++++++++++++------------ 2 files changed, 63 insertions(+), 54 deletions(-) diff --git a/plugins/solidigm/solidigm-nvme.h b/plugins/solidigm/solidigm-nvme.h index e2f416e6..edb1e762 100644 --- a/plugins/solidigm/solidigm-nvme.h +++ b/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( diff --git a/plugins/solidigm/solidigm-telemetry.c b/plugins/solidigm/solidigm-telemetry.c index 871b7530..24926e46 100644 --- a/plugins/solidigm/solidigm-telemetry.c +++ b/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,9 +51,15 @@ 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"; @@ -61,20 +67,25 @@ int solidigm_get_telemetry_log(int argc, char **argv, struct command *cmd, struc 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 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) = { @@ -82,55 +93,61 @@ int solidigm_get_telemetry_log(int argc, char **argv, struct command *cmd, struc 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, @@ -138,12 +155,13 @@ int solidigm_get_telemetry_log(int argc, char **argv, struct command *cmd, struc 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; } -- 2.50.1