Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: > 24.06.2020 19:43, Markus Armbruster wrote: >> Replace >> >> error_setg(&err, ...); >> error_propagate(errp, err); >> >> by >> >> error_setg(errp, ...); >> >> Related pattern: >> >> if (...) { >> error_setg(&err, ...); >> goto out; >> } >> ... >> out: >> error_propagate(errp, err); >> return; >> >> When all paths to label out are that way, replace by >> >> if (...) { >> error_setg(errp, ...); >> return; >> } >> >> and delete the label along with the error_propagate(). >> >> When we have at most one other path that actually needs to propagate, >> and maybe one at the end that where propagation is unnecessary, e.g. >> >> foo(..., &err); >> if (err) { >> goto out; >> } >> ... >> bar(..., &err); >> out: >> error_propagate(errp, err); >> return; >> >> move the error_propagate() to where it's needed, like >> >> if (...) { >> foo(..., &err); >> error_propagate(errp, err); >> return; >> } >> ... >> bar(..., errp); >> return; >> >> and transform the error_setg() as above. >> >> Bonus: the elimination of gotos will make later patches in this series >> easier to review. >> >> Candidates for conversion tracked down with this Coccinelle script: >> >> @@ >> identifier err, errp; >> expression list args; >> @@ >> - error_setg(&err, args); >> + error_setg(errp, args); >> ... when != err >> error_propagate(errp, err); >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> backends/cryptodev.c | 11 +++--- >> backends/hostmem-file.c | 19 +++------- >> backends/hostmem-memfd.c | 15 ++++---- >> backends/hostmem.c | 27 ++++++-------- >> block/throttle-groups.c | 22 +++++------ >> hw/hyperv/vmbus.c | 5 +-- >> hw/i386/pc.c | 35 ++++++------------ >> hw/mem/nvdimm.c | 17 ++++----- >> hw/mem/pc-dimm.c | 14 +++---- >> hw/misc/aspeed_sdmc.c | 3 +- >> hw/ppc/rs6000_mc.c | 9 ++--- >> hw/ppc/spapr.c | 73 ++++++++++++++++--------------------- >> hw/ppc/spapr_pci.c | 14 +++---- >> hw/s390x/ipl.c | 23 +++++------- >> hw/s390x/sclp.c | 12 ++---- >> hw/xen/xen_pt_config_init.c | 3 +- >> iothread.c | 12 +++--- >> net/colo-compare.c | 20 ++++------ >> net/dump.c | 10 ++--- >> net/filter-buffer.c | 10 ++--- >> qga/commands-win32.c | 16 +++----- >> 21 files changed, 151 insertions(+), 219 deletions(-) >> > > [..] > >> --- a/qga/commands-win32.c >> +++ b/qga/commands-win32.c >> @@ -282,9 +282,8 @@ static void execute_async(DWORD WINAPI (*func)(LPVOID), >> LPVOID opaque, > > You forget to remove unused local_err variable
In my haste to get this series out for review, I took undadvisable shortcuts on Error * variable cleanup. Need to do better for v2. >> HANDLE thread = CreateThread(NULL, 0, func, opaque, 0, >> NULL); >> if (!thread) { >> - error_setg(&local_err, QERR_QGA_COMMAND_FAILED, >> + error_setg(errp, QERR_QGA_COMMAND_FAILED, >> "failed to dispatch asynchronous command"); >> - error_propagate(errp, local_err); >> } >> } >> @@ -1274,31 +1273,28 @@ static void >> check_suspend_mode(GuestSuspendMode mode, Error **errp) > > and here (I assume, you remove unused variables with help of compiler, but > don't compile for win32 :) > > > with these two local_err removed: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> Thanks! > Ohh, my brain is broken, I'd prefer to create such patches than to review > them :) Rrrrrevenge! ;) [...]