Hi
On Mon, Oct 8, 2018 at 9:57 PM Markus Armbruster <arm...@redhat.com> wrote:
>
> Calling error_report() in a function that takes an Error ** argument
> is suspicious. parse_fw_cfg() does that, and then fails without
> setting an error. Its caller main(), via qemu_opts_foreach(), is fine
> with it, but clean it up anyway.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> vl.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 1009d708a0..a3a39ec06b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2174,7 +2174,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
> Error **errp)
> FWCfgState *fw_cfg = (FWCfgState *) opaque;
>
> if (fw_cfg == NULL) {
> - error_report("fw_cfg device not available");
> + error_setg(errp, "fw_cfg device not available");
> return -1;
> }
> name = qemu_opt_get(opts, "name");
> @@ -2183,15 +2183,16 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
> Error **errp)
>
> /* we need name and either a file or the content string */
> if (!(nonempty_str(name) && (nonempty_str(file) || nonempty_str(str)))) {
> - error_report("invalid argument(s)");
> + error_setg(errp, "invalid argument(s)");
> return -1;
> }
> if (nonempty_str(file) && nonempty_str(str)) {
> - error_report("file and string are mutually exclusive");
> + error_setg(errp, "file and string are mutually exclusive");
> return -1;
> }
> if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
> - error_report("name too long (max. %d char)", FW_CFG_MAX_FILE_PATH -
> 1);
> + error_setg(errp, "name too long (max. %d char)",
> + FW_CFG_MAX_FILE_PATH - 1);
> return -1;
> }
> if (strncmp(name, "opt/", 4) != 0) {
> @@ -2203,7 +2204,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
> Error **errp)
> buf = g_memdup(str, size);
> } else {
> if (!g_file_get_contents(file, &buf, &size, NULL)) {
> - error_report("can't load %s", file);
> + error_setg(errp, "can't load %s", file);
> return -1;
> }
> }
> @@ -4429,10 +4430,8 @@ int main(int argc, char **argv, char **envp)
> hax_sync_vcpus();
> }
>
> - if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
> - parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
> - exit(1);
> - }
> + qemu_opts_foreach(qemu_find_opts("fw_cfg"),
> + parse_fw_cfg, fw_cfg_find(), &error_fatal);
>
> /* init USB devices */
> if (machine_usb(current_machine)) {
> --
> 2.17.1
>
>
--
Marc-André Lureau
