From 077195187b47d83418e5fb521c89d7881fab3049 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 20 Jul 2020 14:20:15 +0200 Subject: [PATCH] hw/nvram/fw_cfg: Let fw_cfg_add_from_generator() return boolean value MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Commits b6d7e9b66f..a43770df5d simplified the error propagation. Similarly to commit 6fd5bef10b "qom: Make functions taking Error** return bool, not void", let fw_cfg_add_from_generator() return a boolean value, not void. This allow to simplify parse_fw_cfg() and fixes the error handling issue reported by Coverity (CID 1430396): In parse_fw_cfg(): Variable assigned once to a constant guards dead code. Local variable local_err is assigned only once, to a constant value, making it effectively constant throughout its scope. If this is not the intent, examine the logic to see if there is a missing assignment that would make local_err not remain constant. It's the call of fw_cfg_add_from_generator(): Error *local_err = NULL; fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp); if (local_err) { error_propagate(errp, local_err); return -1; } return 0; If it fails, parse_fw_cfg() sets an error and returns 0, which is wrong. Harmless, because the only caller passes &error_fatal. Reported-by: Peter Maydell Fixes: Coverity CID 1430396: 'Constant' variable guards dead code (DEADCODE) Fixes: 6552d87c48 ("softmmu/vl: Let -fw_cfg option take a 'gen_id' argument") Reviewed-by: Laszlo Ersek Reviewed-by: Markus Armbruster Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20200721131911.27380-3-philmd@redhat.com> --- hw/nvram/fw_cfg.c | 10 ++++++---- include/hw/nvram/fw_cfg.h | 4 +++- softmmu/vl.c | 6 +----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index dfa1f2012a..f3a4728288 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -1032,7 +1032,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, return NULL; } -void fw_cfg_add_from_generator(FWCfgState *s, const char *filename, +bool fw_cfg_add_from_generator(FWCfgState *s, const char *filename, const char *gen_id, Error **errp) { FWCfgDataGeneratorClass *klass; @@ -1043,20 +1043,22 @@ void fw_cfg_add_from_generator(FWCfgState *s, const char *filename, obj = object_resolve_path_component(object_get_objects_root(), gen_id); if (!obj) { error_setg(errp, "Cannot find object ID '%s'", gen_id); - return; + return false; } if (!object_dynamic_cast(obj, TYPE_FW_CFG_DATA_GENERATOR_INTERFACE)) { error_setg(errp, "Object ID '%s' is not a '%s' subclass", gen_id, TYPE_FW_CFG_DATA_GENERATOR_INTERFACE); - return; + return false; } klass = FW_CFG_DATA_GENERATOR_GET_CLASS(obj); array = klass->get_data(obj, errp); if (!array) { - return; + return false; } size = array->len; fw_cfg_add_file(s, filename, g_byte_array_free(array, TRUE), size); + + return true; } static void fw_cfg_machine_reset(void *opaque) diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index bbcf405649..f190c428e8 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -304,8 +304,10 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data, * will be used; also, a new entry will be added to the file directory * structure residing at key value FW_CFG_FILE_DIR, containing the item name, * data size, and assigned selector key value. + * + * Returns: %true on success, %false on error. */ -void fw_cfg_add_from_generator(FWCfgState *s, const char *filename, +bool fw_cfg_add_from_generator(FWCfgState *s, const char *filename, const char *gen_id, Error **errp); FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, diff --git a/softmmu/vl.c b/softmmu/vl.c index f476ef89ed..3416241557 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2070,11 +2070,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp) size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */ buf = g_memdup(str, size); } else if (nonempty_str(gen_id)) { - Error *local_err = NULL; - - fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp); - if (local_err) { - error_propagate(errp, local_err); + if (!fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp)) { return -1; } return 0; -- 2.11.4.GIT