On Mon, 20 Jul 2020 17:24:35 +0200 Markus Armbruster <arm...@redhat.com> wrote:
> Greg Kurz <gr...@kaod.org> writes: > > > We have a dedicated error API for hints. Use it instead of embedding > > the hint in the error message, as recommanded in the "qapi/error.h" > > header file. > > > > Since spapr_caps_apply() passes &error_fatal, all functions must > > also call the ERRP_GUARD() macro for error_append_hint() to be > > functional. > > This isn't a request for change in this patch, just an attempt to squash > possible misunderstandings. > > It's true that error_append_hint() without ERRP_GUARD() works as long as > the caller doesn't pass certain errp arguments. But the callee should > work for all possible @errp arguments, not just the ones that get passed > today. That's why error.h wants you to guard *all* uses of > error_append_hint(errp): > > * = Why, when and how to use ERRP_GUARD() = > * > * Without ERRP_GUARD(), use of the @errp parameter is restricted: > * - It must not be dereferenced, because it may be null. > * - It should not be passed to error_prepend() or > * error_append_hint(), because that doesn't work with &error_fatal. > * ERRP_GUARD() lifts these restrictions. > Yeah, I just wanted to emphasize that we were precisely in the case where we _really_ need to lift the restriction, but I'm perfectly fine with dropping this sentence if you consider it useless. BTW, should we have a way for CI to ensure that a patch that adds error_prepend(errp, ...) or error_append_hint(errp, ...) also adds ERRP_GUARD() ? Not sure that people read error.h that often... > No need to make an argument involving the possible arguments (pardon the > pun). > :) > [...] >
