Markus Armbruster <arm...@redhat.com> writes:
> Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes:
>
>> 24.06.2020 19:43, Markus Armbruster wrote:
>>> When all we do with an Error we receive into a local variable is
>>> propagating to somewhere else, we can just as well receive it there
>>> right away. The previous commit did that for simple cases with
>>> Coccinelle. Do it for a few more manually.
>>>
>>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>>> ---
>>> blockdev.c | 5 +----
>>> hw/core/numa.c | 44 ++++++++++++++------------------------------
>>> qdev-monitor.c | 11 ++++-------
>>> 3 files changed, 19 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/blockdev.c b/blockdev.c
>>> index b66863c42a..73736a4eaf 100644
>>> --- a/blockdev.c
>>> +++ b/blockdev.c
>>> @@ -1009,13 +1009,10 @@ DriveInfo *drive_new(QemuOpts *all_opts,
>>> BlockInterfaceType block_default_type,
>>> }
>>> /* Actual block device init: Functionality shared with
>>> blockdev-add */
>>> - blk = blockdev_init(filename, bs_opts, &local_err);
>>> + blk = blockdev_init(filename, bs_opts, errp);
>>> bs_opts = NULL;
>>> if (!blk) {
>>> - error_propagate(errp, local_err);
>>> goto fail;
>>> - } else {
>>> - assert(!local_err);
>>> }
>>> /* Create legacy DriveInfo */
>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>> index 5f81900f88..aa8c6be210 100644
>>> --- a/hw/core/numa.c
>>> +++ b/hw/core/numa.c
>>> @@ -449,40 +449,33 @@ void parse_numa_hmat_cache(MachineState *ms,
>>> NumaHmatCacheOptions *node,
>>> void set_numa_options(MachineState *ms, NumaOptions *object,
>>> Error **errp)
>>> {
>>> - Error *err = NULL;
>>> -
>>> if (!ms->numa_state) {
>>> error_setg(errp, "NUMA is not supported by this machine-type");
>>> - goto end;
>>> + return;
>>> }
>>> switch (object->type) {
>>> case NUMA_OPTIONS_TYPE_NODE:
>>> - parse_numa_node(ms, &object->u.node, &err);
>>> - if (err) {
>>> - goto end;
>>> - }
>>> + parse_numa_node(ms, &object->u.node, errp);
>>> break;
>>
>> Could we use return here and and for other "break" operators here, to
>> stress, that we
>> are not going to do something more in case of failure (as well as in case of
>> success)? To prevent the future addition of some code after the switch
>> without
>> handling the error carefully here.
>
> Can do.
Second thoughts: I'd prefer not to mess with it now.
The sane way to add code after the switch is to make the
parse_numa_FOO() return bool, then bail out like this:
if (!parse_numa_node(ms, &object->u.node, errp)) {
return;
}
Too much for me right now. I'm having a hard time getting this ready in
time of the freeze. We can always improve on top.
[...]
