19.09.2019 11:59, Max Reitz wrote: > On 18.09.19 15:02, Vladimir Sementsov-Ogievskiy wrote: >> Hi all! >> >> Here is a proposal (three of them, actually) of auto propagation for >> local_err, to not call error_propagate on every exit point, when we >> deal with local_err. >> >> It also may help make Greg's series[1] about error_append_hint smaller. >> >> See definitions and examples below. >> >> I'm cc-ing to this RFC everyone from series[1] CC list, as if we like >> it, the idea will touch same code (and may be more). >> >> [1]: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg03449.html >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> include/qapi/error.h | 102 +++++++++++++++++++++++++++++++++++++++++++ >> block.c | 63 ++++++++++++-------------- >> block/backup.c | 8 +++- >> block/gluster.c | 7 +++ >> 4 files changed, 144 insertions(+), 36 deletions(-) > > If the combination of “if (local_err) { error_propagate(...); ... }” is > what’s cumbersome, can’t this be done simpler by adding an > error_propagate() variant with a return value? > > i.e. > > bool has_error_then_propagate(Error **errp, Error *err) > { > if (!err) { > return false; > } > error_propagate(errp, err); > return true; > } > > And then turn all instances of > > if (local_err) { > error_propagate(errp, local_err); > ... > } > > into > > if (has_error_then_propagate(errp, local_err)) { > ... > } > > ? > > Max >
No, originally cumbersome is introducing local_err in a lot of new places by Greg's series. MAKE_ERRP_SAFE macro makes it as simple as one macro call instead (in each function where we need local_err). Also, auto-propagation seems correct thing to do, which fits good into g_auto concept, so even without any macro it just allows to drop several error_propagate calls (or may be several goto statements to do one error_propagate call) into one definitions. It's the same story like with g_autofree vs g_free. -- Best regards, Vladimir