new file mode 100644
@@ -0,0 +1,336 @@
+// Use ERRP_GUARD() (see include/qapi/error.h)
+//
+// Copyright (c) 2020 Virtuozzo International GmbH.
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License as
+// published by the Free Software Foundation; either version 2 of the
+// License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see
+// <http://www.gnu.org/licenses/>.
+//
+// Usage example:
+// spatch --sp-file scripts/coccinelle/errp-guard.cocci \
+// --macro-file scripts/cocci-macro-file.h --in-place \
+// --no-show-diff --max-width 80 FILES...
+//
+// Note: --max-width 80 is needed because coccinelle default is less
+// than 80, and without this parameter coccinelle may reindent some
+// lines which fit into 80 characters but not to coccinelle default,
+// which in turn produces extra patch hunks for no reason.
+
+// Switch unusual Error ** parameter names to errp
+// (this is necessary to use ERRP_GUARD).
+//
+// Disable optional_qualifier to skip functions with
+// "Error *const *errp" parameter.
+//
+// Skip functions with "assert(_errp && *_errp)" statement, because
+// that signals unusual semantics, and the parameter name may well
+// serve a purpose. (like nbd_iter_channel_error()).
+//
+// Skip util/error.c to not touch, for example, error_propagate() and
+// error_propagate_prepend().
+@ depends on !(file in "util/error.c") disable optional_qualifier@
+identifier fn;
+identifier _errp != errp;
+@@
+
+ fn(...,
+- Error **_errp
++ Error **errp
+ ,...)
+ {
+(
+ ... when != assert(_errp && *_errp)
+&
+ <...
+- _errp
++ errp
+ ...>
+)
+ }
+
+// Add invocation of ERRP_GUARD() to errp-functions where // necessary
+//
+// Note, that without "when any" the final "..." does not mach
+// something matched by previous pattern, i.e. the rule will not match
+// double error_prepend in control flow like in
+// vfio_set_irq_signaling().
+//
+// Note, "exists" says that we want apply rule even if it does not
+// match on all possible control flows (otherwise, it will not match
+// standard pattern when error_propagate() call is in if branch).
+@ disable optional_qualifier exists@
+identifier fn, local_err;
+symbol errp;
+@@
+
+ fn(..., Error **errp, ...)
+ {
++ ERRP_GUARD();
+ ... when != ERRP_GUARD();
+(
+(
+ error_append_hint(errp, ...);
+|
+ error_prepend(errp, ...);
+|
+ error_vprepend(errp, ...);
+)
+ ... when any
+|
+ Error *local_err = NULL;
+ ...
+(
+ error_propagate_prepend(errp, local_err, ...);
+|
+ error_propagate(errp, local_err);
+)
+ ...
+)
+ }
+
+// Warn when several Error * definitions are in the control flow.
+// This rule is not chained to rule1 and less restrictive, to cover more
+// functions to warn (even those we are not going to convert).
+//
+// Note, that even with one (or zero) Error * definition in the each
+// control flow we may have several (in total) Error * definitions in
+// the function. This case deserves attention too, but I don't see
+// simple way to match with help of coccinelle.
+@check1 disable optional_qualifier exists@
+identifier fn, _errp, local_err, local_err2;
+position p1, p2;
+@@
+
+ fn(..., Error **_errp, ...)
+ {
+ ...
+ Error *local_err = NULL;@p1
+ ... when any
+ Error *local_err2 = NULL;@p2
+ ... when any
+ }
+
+@ script:python @
+fn << check1.fn;
+p1 << check1.p1;
+p2 << check1.p2;
+@@
+
+print('Warning: function {} has several definitions of '
+ 'Error * local variable: at {}:{} and then at {}:{}'.format(
+ fn, p1[0].file, p1[0].line, p2[0].file, p2[0].line))
+
+// Warn when several propagations are in the control flow.
+@check2 disable optional_qualifier exists@
+identifier fn, _errp;
+position p1, p2;
+@@
+
+ fn(..., Error **_errp, ...)
+ {
+ ...
+(
+ error_propagate_prepend(_errp, ...);@p1
+|
+ error_propagate(_errp, ...);@p1
+)
+ ...
+(
+ error_propagate_prepend(_errp, ...);@p2
+|
+ error_propagate(_errp, ...);@p2
+)
+ ... when any
+ }
+
+@ script:python @
+fn << check2.fn;
+p1 << check2.p1;
+p2 << check2.p2;
+@@
+
+print('Warning: function {} propagates to errp several times in '
+ 'one control flow: at {}:{} and then at {}:{}'.format(
+ fn, p1[0].file, p1[0].line, p2[0].file, p2[0].line))
+
+// Match functions with propagation of local error to errp.
+// We want to refer these functions in several following rules, but I
+// don't know a proper way to inherit a function, not just its name
+// (to not match another functions with same name in following rules).
+// Not-proper way is as follows: rename errp parameter in functions
+// header and match it in following rules. Rename it back after all
+// transformations.
+//
+// The common case is a single definition of local_err with at most one
+// error_propagate_prepend() or error_propagate() on each control-flow
+// path. Functions with multiple definitions or propagates we want to
+// examine manually. Rules check1 and check2 emit warnings to guide us
+// to them.
+//
+// Note that we match not only this "common case", but any function,
+// which has the "common case" on at least one control-flow path.
+@rule1 disable optional_qualifier exists@
+identifier fn, local_err;
+symbol errp;
+@@
+
+ fn(..., Error **
+- errp
++ ____
+ , ...)
+ {
+ ...
+ Error *local_err = NULL;
+ ...
+(
+ error_propagate_prepend(errp, local_err, ...);
+|
+ error_propagate(errp, local_err);
+)
+ ...
+ }
+
+// Convert special case with goto separately.
+// I tried merging this into the following rule the obvious way, but
+// it made Coccinelle hang on block.c
+//
+// Note interesting thing: if we don't do it here, and try to fixup
+// "out: }" things later after all transformations (the rule will be
+// the same, just without error_propagate() call), coccinelle fails to
+// match this "out: }".
+@ disable optional_qualifier@
+identifier rule1.fn, rule1.local_err, out;
+symbol errp;
+@@
+
+ fn(..., Error ** ____, ...)
+ {
+ <...
+- goto out;
++ return;
+ ...>
+- out:
+- error_propagate(errp, local_err);
+ }
+
+// Convert most of local_err related stuff.
+//
+// Note, that we inherit rule1.fn and rule1.local_err names, not
+// objects themselves. We may match something not related to the
+// pattern matched by rule1. For example, local_err may be defined with
+// the same name in different blocks inside one function, and in one
+// block follow the propagation pattern and in other block doesn't.
+//
+// Note also that errp-cleaning functions
+// error_free_errp
+// error_report_errp
+// error_reportf_errp
+// warn_report_errp
+// warn_reportf_errp
+// are not yet implemented. They must call corresponding Error* -
+// freeing function and then set *errp to NULL, to avoid further
+// propagation to original errp (consider ERRP_GUARD in use).
+// For example, error_free_errp may look like this:
+//
+// void error_free_errp(Error **errp)
+// {
+// error_free(*errp);
+// *errp = NULL;
+// }
+@ disable optional_qualifier exists@
+identifier rule1.fn, rule1.local_err;
+expression list args;
+symbol errp;
+@@
+
+ fn(..., Error ** ____, ...)
+ {
+ <...
+(
+- Error *local_err = NULL;
+|
+
+// Convert error clearing functions
+(
+- error_free(local_err);
++ error_free_errp(errp);
+|
+- error_report_err(local_err);
++ error_report_errp(errp);
+|
+- error_reportf_err(local_err, args);
++ error_reportf_errp(errp, args);
+|
+- warn_report_err(local_err);
++ warn_report_errp(errp);
+|
+- warn_reportf_err(local_err, args);
++ warn_reportf_errp(errp, args);
+)
+?- local_err = NULL;
+
+|
+- error_propagate_prepend(errp, local_err, args);
++ error_prepend(errp, args);
+|
+- error_propagate(errp, local_err);
+|
+- &local_err
++ errp
+)
+ ...>
+ }
+
+// Convert remaining local_err usage. For example, different kinds of
+// error checking in if conditionals. We can't merge this into
+// previous hunk, as this conflicts with other substitutions in it (at
+// least with "- local_err = NULL").
+@ disable optional_qualifier@
+identifier rule1.fn, rule1.local_err;
+symbol errp;
+@@
+
+ fn(..., Error ** ____, ...)
+ {
+ <...
+- local_err
++ *errp
+ ...>
+ }
+
+// Always use the same pattern for checking error
+@ disable optional_qualifier@
+identifier rule1.fn;
+symbol errp;
+@@
+
+ fn(..., Error ** ____, ...)
+ {
+ <...
+- *errp != NULL
++ *errp
+ ...>
+ }
+
+// Revert temporary ___ identifier.
+@ disable optional_qualifier@
+identifier rule1.fn;
+@@
+
+ fn(..., Error **
+- ____
++ errp
+ , ...)
+ {
+ ...
+ }
@@ -265,6 +265,8 @@
* }
* ...
* }
+ *
+ * For mass-conversion, use scripts/coccinelle/errp-guard.cocci.
*/
#ifndef ERROR_H
@@ -2176,6 +2176,7 @@ F: scripts/coccinelle/error-use-after-free.cocci
F: scripts/coccinelle/error_propagate_null.cocci
F: scripts/coccinelle/remove_local_err.cocci
F: scripts/coccinelle/use-error_fatal.cocci
+F: scripts/coccinelle/errp-guard.cocci
GDB stub
M: Alex Bennée <alex.bennee@linaro.org>