Daniel P. Berrangé <[email protected]> writes: > On Fri, Sep 18, 2026 at 07:29:54PM -0300, Fabiano Rosas wrote: >> The qemu_opts_parse, qemu_opts_parse_list and qemu_opts_parse_noisily >> functions all call opts_parse(), they can reuse code. >> >> Consolidate them at qemu_opts_parse_list(). Use the presence of the >> Error pointer to determine whether the caller wants to be noisy: if >> there is an Error pointer, then use it, otherwise print the message. >> >> This makes the code more opaque, so keep the qemu_opts_parse and >> qemu_opts_parse_noisily functions as wrappers to make sure the intent >> at the callers is still clear. > > It is also somewhat differnt from the normal semantics in QEMU. > > The behaviour of passing NULL for an "Error **errp" parameter > indicates that the errors are to be ignored/discarded. > > This patch introduces atypical semantics, that the errors must > be printed, so I'm not entirely convinced this patch is a win. >
I understand. This is the main reason why this series is RFC. I could exchange the implicit "pass NULL to mean noisily" with an explicit parameter 'bool noisily'. >> >> Signed-off-by: Fabiano Rosas <[email protected]> >> --- >> tests/unit/test-opts-visitor.c | 2 +- >> util/qemu-option.c | 32 ++++++++++++++++++-------------- >> 2 files changed, 19 insertions(+), 15 deletions(-) > >> diff --git a/util/qemu-option.c b/util/qemu-option.c >> index bb38d305c9..c8e552823b 100644 >> --- a/util/qemu-option.c >> +++ b/util/qemu-option.c >> @@ -938,7 +938,23 @@ QemuOpts *qemu_opts_parse(const char *group, const char >> *params, >> QemuOpts *qemu_opts_parse_list(QemuOptsList *list, const char *params, >> bool permit_abbrev, Error **errp) >> { >> - return opts_parse(list, params, permit_abbrev, false, false, errp); >> + bool noisily = !errp; >> + QemuOpts *opts = NULL; >> + >> + if (noisily) { >> + Error *err = NULL; >> + bool help_wanted = !opts_accepts_any(list); >> + >> + opts = opts_parse(list, params, permit_abbrev, true, help_wanted, >> &err); >> + if (!opts && err) { >> + error_report_err(err); >> + } >> + >> + } else { >> + opts = opts_parse(list, params, permit_abbrev, false, false, errp); >> + } >> + >> + return opts; >> } >> >> /** >> @@ -952,19 +968,7 @@ QemuOpts *qemu_opts_parse_list(QemuOptsList *list, >> const char *params, >> QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params, >> bool permit_abbrev) >> { >> - Error *err = NULL; >> - QemuOpts *opts; >> - bool help_wanted = !opts_accepts_any(list); >> - >> - opts = opts_parse(list, params, permit_abbrev, true, help_wanted, &err); >> - if (!opts) { >> - if (help_wanted) { >> - assert(!err); >> - } else { >> - error_report_err(err); >> - } >> - } >> - return opts; >> + return qemu_opts_parse_list(list, params, permit_abbrev, NULL); >> } >> >> static bool qemu_opts_from_qdict_entry(QemuOpts *opts, >> -- >> 2.53.0 >> >> > > With regards, > Daniel
