Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: > 08.10.2019 12:08, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: >> >>> Error **errp is almost always OUT-argument: it's assumed to be NULL, or >>> pointer to NULL-initialized pointer, or pointer to error_abort or >>> error_fatal, for callee to report error. >> >> Yes. >> >>> But very few functions instead get Error **errp as IN-argument: >>> it's assumed to be set, and callee should clean it. >> >> What do you mean by "callee should clean"? Let's see below. >> >>> In such cases, rename errp to errp_in. >> >> I acknowledge that errp arguments that don't have the usual meaning can >> be confusing. >> >> Naming can help, comments can help, but perhaps we can tweak the code to >> avoid the problem instead. Let's see: >> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >>> Reviewed-by: Eric Blake <ebl...@redhat.com> [...] >> We can avoid the confusing Error **errp in all three cases by peeling >> off an indirection. What do you think? >> > > I like the idea, thanks! I think, I'll try to make a patch. > > But you are right, unfortunately there more cases, at least, pointed by > Greg > > error_append_socket_sockfd_hint and > error_append_security_model_hint > > Which don't free error..
Neither do error_append_hint() and error_prepend(). For anything named error_append_FOO_hint() or error_prepend_FOO(), confusion seems unlikely. > But if they append hint, they should always be called > on wrapped errp, accordingly to the problem about fatal_error, so they may > be converted to Error *err too.. Hmm, I should think about the script to find > such functions. I figure I better read more of your series before I comment on this thought.
