scripts/coccinelle/errp-guard.cocci

// 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
    , ...)
 {
     ...
 }