Fabiano Rosas <[email protected]> writes:
> Fabiano Rosas <[email protected]> writes:
>
>> Markus Armbruster <[email protected]> writes:
[...]
>>> I think a hmp_report_error(MonitorHMP *hmp, const char *fmt, ...) would
>>> be nice to have.
>>>
>>
>> I'll look into it.
>>
>
> I see that hmp_handle_error() takes an hmp argument that's unused and
Unused since commit 193227f9e5 (error: Use error_report_err() instead of
monitor_printf(), 2015-12-18).
I doubt it's worth removing.
> also that callers often do the redundant:
>
> if (err) {
> hmp_handle_error(err);
> }
Would be nice to clean this up.
> But aside from those I don't see other improvements to be made, many
> callsites of hmp_handle_error() re-use the Error for other calls,
> e.g. block/monitor/block-hmp-cmds.c:
>
> Error *err = NULL;
> ...
> if (!qdict_get_try_str(qdict, "node-name")) {
> qobject_unref(qdict);
> error_setg(&err, "'node-name' needs to be specified");
> goto out;
> }
>
> BlockDriverState *bs = bds_tree_init(qdict, &err);
> if (!bs) {
> goto out;
> }
>
> bdrv_set_monitor_owned(bs);
> out:
> qemu_opts_del(opts);
> hmp_handle_error(err);
>
> So having another helper I think would just create confusion as
> error_setg() is already a standard way of creating an error.
We have both error_report() and error_report_err(), and it doesn't lead
to confusion as far as I can tell.
Whether doing the same for HMP error reporting would be an improvement
is not obvious. Let's move on.
[...]
