Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: > 08.10.2019 12:08, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: [...] >>> diff --git a/util/error.c b/util/error.c >>> index d4532ce318..b3ff3832d6 100644 >>> --- a/util/error.c >>> +++ b/util/error.c >>> @@ -271,11 +271,11 @@ void error_free(Error *err) >>> } >>> } >>> >>> -void error_free_or_abort(Error **errp) >>> +void error_free_or_abort(Error **errp_in) >>> { >>> - assert(errp && *errp); >>> - error_free(*errp); >>> - *errp = NULL; >>> + assert(errp_in && *errp_in); >>> + error_free(*errp_in); >>> + *errp_in = NULL; >>> } >>> >>> void error_propagate(Error **dst_errp, Error *local_err) >> >> This functions frees the error. It leaves nothing for the caller to >> clean up. >> >> All callers pass &ERR, where ERR is a local variable. We can peel off >> an indirection. > > > But if we drop indirection, we'll have to set local variable to NULL by > hand. Is it good? > > Look at test_keyval_parse_list() for example: it uses local err object > several times, so it depends on the fact that error_free_or_abort > sets pointer to NULL.
You're right, peeling off the indirection would make error_free_or_abort() worse. It's a convenience function for tests. Confusion seems unlikely to me. Let's not worry about it.