| 1 | // Use ERRP_GUARD() (see include/qapi/error.h) |
| 2 | // |
| 3 | // Copyright (c) 2020 Virtuozzo International GmbH. |
| 4 | // |
| 5 | // This program is free software; you can redistribute it and/or |
| 6 | // modify it under the terms of the GNU General Public License as |
| 7 | // published by the Free Software Foundation; either version 2 of the |
| 8 | // License, or (at your option) any later version. |
| 9 | // |
| 10 | // This program is distributed in the hope that it will be useful, |
| 11 | // but WITHOUT ANY WARRANTY; without even the implied warranty of |
| 12 | // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
| 13 | // GNU General Public License for more details. |
| 14 | // |
| 15 | // You should have received a copy of the GNU General Public License |
| 16 | // along with this program. If not, see |
| 17 | // <http://www.gnu.org/licenses/>. |
| 18 | // |
| 19 | // Usage example: |
| 20 | // spatch --sp-file scripts/coccinelle/errp-guard.cocci \ |
| 21 | // --macro-file scripts/cocci-macro-file.h --in-place \ |
| 22 | // --no-show-diff --max-width 80 FILES... |
| 23 | // |
| 24 | // Note: --max-width 80 is needed because coccinelle default is less |
| 25 | // than 80, and without this parameter coccinelle may reindent some |
| 26 | // lines which fit into 80 characters but not to coccinelle default, |
| 27 | // which in turn produces extra patch hunks for no reason. |
| 28 | |
| 29 | // Switch unusual Error ** parameter names to errp |
| 30 | // (this is necessary to use ERRP_GUARD). |
| 31 | // |
| 32 | // Disable optional_qualifier to skip functions with |
| 33 | // "Error *const *errp" parameter. |
| 34 | // |
| 35 | // Skip functions with "assert(_errp && *_errp)" statement, because |
| 36 | // that signals unusual semantics, and the parameter name may well |
| 37 | // serve a purpose. (like nbd_iter_channel_error()). |
| 38 | // |
| 39 | // Skip util/error.c to not touch, for example, error_propagate() and |
| 40 | // error_propagate_prepend(). |
| 41 | @ depends on !(file in "util/error.c") disable optional_qualifier@ |
| 42 | identifier fn; |
| 43 | identifier _errp != errp; |
| 44 | @@ |
| 45 | |
| 46 | fn(..., |
| 47 | - Error **_errp |
| 48 | + Error **errp |
| 49 | ,...) |
| 50 | { |
| 51 | ( |
| 52 | ... when != assert(_errp && *_errp) |
| 53 | & |
| 54 | <... |
| 55 | - _errp |
| 56 | + errp |
| 57 | ...> |
| 58 | ) |
| 59 | } |
| 60 | |
| 61 | // Add invocation of ERRP_GUARD() to errp-functions where // necessary |
| 62 | // |
| 63 | // Note, that without "when any" the final "..." does not mach |
| 64 | // something matched by previous pattern, i.e. the rule will not match |
| 65 | // double error_prepend in control flow like in |
| 66 | // vfio_set_irq_signaling(). |
| 67 | // |
| 68 | // Note, "exists" says that we want apply rule even if it does not |
| 69 | // match on all possible control flows (otherwise, it will not match |
| 70 | // standard pattern when error_propagate() call is in if branch). |
| 71 | @ disable optional_qualifier exists@ |
| 72 | identifier fn, local_err; |
| 73 | symbol errp; |
| 74 | @@ |
| 75 | |
| 76 | fn(..., Error **errp, ...) |
| 77 | { |
| 78 | + ERRP_GUARD(); |
| 79 | ... when != ERRP_GUARD(); |
| 80 | ( |
| 81 | ( |
| 82 | error_append_hint(errp, ...); |
| 83 | | |
| 84 | error_prepend(errp, ...); |
| 85 | | |
| 86 | error_vprepend(errp, ...); |
| 87 | ) |
| 88 | ... when any |
| 89 | | |
| 90 | Error *local_err = NULL; |
| 91 | ... |
| 92 | ( |
| 93 | error_propagate_prepend(errp, local_err, ...); |
| 94 | | |
| 95 | error_propagate(errp, local_err); |
| 96 | ) |
| 97 | ... |
| 98 | ) |
| 99 | } |
| 100 | |
| 101 | // Warn when several Error * definitions are in the control flow. |
| 102 | // This rule is not chained to rule1 and less restrictive, to cover more |
| 103 | // functions to warn (even those we are not going to convert). |
| 104 | // |
| 105 | // Note, that even with one (or zero) Error * definition in the each |
| 106 | // control flow we may have several (in total) Error * definitions in |
| 107 | // the function. This case deserves attention too, but I don't see |
| 108 | // simple way to match with help of coccinelle. |
| 109 | @check1 disable optional_qualifier exists@ |
| 110 | identifier fn, _errp, local_err, local_err2; |
| 111 | position p1, p2; |
| 112 | @@ |
| 113 | |
| 114 | fn(..., Error **_errp, ...) |
| 115 | { |
| 116 | ... |
| 117 | Error *local_err = NULL;@p1 |
| 118 | ... when any |
| 119 | Error *local_err2 = NULL;@p2 |
| 120 | ... when any |
| 121 | } |
| 122 | |
| 123 | @ script:python @ |
| 124 | fn << check1.fn; |
| 125 | p1 << check1.p1; |
| 126 | p2 << check1.p2; |
| 127 | @@ |
| 128 | |
| 129 | print('Warning: function {} has several definitions of ' |
| 130 | 'Error * local variable: at {}:{} and then at {}:{}'.format( |
| 131 | fn, p1[0].file, p1[0].line, p2[0].file, p2[0].line)) |
| 132 | |
| 133 | // Warn when several propagations are in the control flow. |
| 134 | @check2 disable optional_qualifier exists@ |
| 135 | identifier fn, _errp; |
| 136 | position p1, p2; |
| 137 | @@ |
| 138 | |
| 139 | fn(..., Error **_errp, ...) |
| 140 | { |
| 141 | ... |
| 142 | ( |
| 143 | error_propagate_prepend(_errp, ...);@p1 |
| 144 | | |
| 145 | error_propagate(_errp, ...);@p1 |
| 146 | ) |
| 147 | ... |
| 148 | ( |
| 149 | error_propagate_prepend(_errp, ...);@p2 |
| 150 | | |
| 151 | error_propagate(_errp, ...);@p2 |
| 152 | ) |
| 153 | ... when any |
| 154 | } |
| 155 | |
| 156 | @ script:python @ |
| 157 | fn << check2.fn; |
| 158 | p1 << check2.p1; |
| 159 | p2 << check2.p2; |
| 160 | @@ |
| 161 | |
| 162 | print('Warning: function {} propagates to errp several times in ' |
| 163 | 'one control flow: at {}:{} and then at {}:{}'.format( |
| 164 | fn, p1[0].file, p1[0].line, p2[0].file, p2[0].line)) |
| 165 | |
| 166 | // Match functions with propagation of local error to errp. |
| 167 | // We want to refer these functions in several following rules, but I |
| 168 | // don't know a proper way to inherit a function, not just its name |
| 169 | // (to not match another functions with same name in following rules). |
| 170 | // Not-proper way is as follows: rename errp parameter in functions |
| 171 | // header and match it in following rules. Rename it back after all |
| 172 | // transformations. |
| 173 | // |
| 174 | // The common case is a single definition of local_err with at most one |
| 175 | // error_propagate_prepend() or error_propagate() on each control-flow |
| 176 | // path. Functions with multiple definitions or propagates we want to |
| 177 | // examine manually. Rules check1 and check2 emit warnings to guide us |
| 178 | // to them. |
| 179 | // |
| 180 | // Note that we match not only this "common case", but any function, |
| 181 | // which has the "common case" on at least one control-flow path. |
| 182 | @rule1 disable optional_qualifier exists@ |
| 183 | identifier fn, local_err; |
| 184 | symbol errp; |
| 185 | @@ |
| 186 | |
| 187 | fn(..., Error ** |
| 188 | - errp |
| 189 | + ____ |
| 190 | , ...) |
| 191 | { |
| 192 | ... |
| 193 | Error *local_err = NULL; |
| 194 | ... |
| 195 | ( |
| 196 | error_propagate_prepend(errp, local_err, ...); |
| 197 | | |
| 198 | error_propagate(errp, local_err); |
| 199 | ) |
| 200 | ... |
| 201 | } |
| 202 | |
| 203 | // Convert special case with goto separately. |
| 204 | // I tried merging this into the following rule the obvious way, but |
| 205 | // it made Coccinelle hang on block.c |
| 206 | // |
| 207 | // Note interesting thing: if we don't do it here, and try to fixup |
| 208 | // "out: }" things later after all transformations (the rule will be |
| 209 | // the same, just without error_propagate() call), coccinelle fails to |
| 210 | // match this "out: }". |
| 211 | @ disable optional_qualifier@ |
| 212 | identifier rule1.fn, rule1.local_err, out; |
| 213 | symbol errp; |
| 214 | @@ |
| 215 | |
| 216 | fn(..., Error ** ____, ...) |
| 217 | { |
| 218 | <... |
| 219 | - goto out; |
| 220 | + return; |
| 221 | ...> |
| 222 | - out: |
| 223 | - error_propagate(errp, local_err); |
| 224 | } |
| 225 | |
| 226 | // Convert most of local_err related stuff. |
| 227 | // |
| 228 | // Note, that we inherit rule1.fn and rule1.local_err names, not |
| 229 | // objects themselves. We may match something not related to the |
| 230 | // pattern matched by rule1. For example, local_err may be defined with |
| 231 | // the same name in different blocks inside one function, and in one |
| 232 | // block follow the propagation pattern and in other block doesn't. |
| 233 | // |
| 234 | // Note also that errp-cleaning functions |
| 235 | // error_free_errp |
| 236 | // error_report_errp |
| 237 | // error_reportf_errp |
| 238 | // warn_report_errp |
| 239 | // warn_reportf_errp |
| 240 | // are not yet implemented. They must call corresponding Error* - |
| 241 | // freeing function and then set *errp to NULL, to avoid further |
| 242 | // propagation to original errp (consider ERRP_GUARD in use). |
| 243 | // For example, error_free_errp may look like this: |
| 244 | // |
| 245 | // void error_free_errp(Error **errp) |
| 246 | // { |
| 247 | // error_free(*errp); |
| 248 | // *errp = NULL; |
| 249 | // } |
| 250 | @ disable optional_qualifier exists@ |
| 251 | identifier rule1.fn, rule1.local_err; |
| 252 | expression list args; |
| 253 | symbol errp; |
| 254 | @@ |
| 255 | |
| 256 | fn(..., Error ** ____, ...) |
| 257 | { |
| 258 | <... |
| 259 | ( |
| 260 | - Error *local_err = NULL; |
| 261 | | |
| 262 | |
| 263 | // Convert error clearing functions |
| 264 | ( |
| 265 | - error_free(local_err); |
| 266 | + error_free_errp(errp); |
| 267 | | |
| 268 | - error_report_err(local_err); |
| 269 | + error_report_errp(errp); |
| 270 | | |
| 271 | - error_reportf_err(local_err, args); |
| 272 | + error_reportf_errp(errp, args); |
| 273 | | |
| 274 | - warn_report_err(local_err); |
| 275 | + warn_report_errp(errp); |
| 276 | | |
| 277 | - warn_reportf_err(local_err, args); |
| 278 | + warn_reportf_errp(errp, args); |
| 279 | ) |
| 280 | ?- local_err = NULL; |
| 281 | |
| 282 | | |
| 283 | - error_propagate_prepend(errp, local_err, args); |
| 284 | + error_prepend(errp, args); |
| 285 | | |
| 286 | - error_propagate(errp, local_err); |
| 287 | | |
| 288 | - &local_err |
| 289 | + errp |
| 290 | ) |
| 291 | ...> |
| 292 | } |
| 293 | |
| 294 | // Convert remaining local_err usage. For example, different kinds of |
| 295 | // error checking in if conditionals. We can't merge this into |
| 296 | // previous hunk, as this conflicts with other substitutions in it (at |
| 297 | // least with "- local_err = NULL"). |
| 298 | @ disable optional_qualifier@ |
| 299 | identifier rule1.fn, rule1.local_err; |
| 300 | symbol errp; |
| 301 | @@ |
| 302 | |
| 303 | fn(..., Error ** ____, ...) |
| 304 | { |
| 305 | <... |
| 306 | - local_err |
| 307 | + *errp |
| 308 | ...> |
| 309 | } |
| 310 | |
| 311 | // Always use the same pattern for checking error |
| 312 | @ disable optional_qualifier@ |
| 313 | identifier rule1.fn; |
| 314 | symbol errp; |
| 315 | @@ |
| 316 | |
| 317 | fn(..., Error ** ____, ...) |
| 318 | { |
| 319 | <... |
| 320 | - *errp != NULL |
| 321 | + *errp |
| 322 | ...> |
| 323 | } |
| 324 | |
| 325 | // Revert temporary ___ identifier. |
| 326 | @ disable optional_qualifier@ |
| 327 | identifier rule1.fn; |
| 328 | @@ |
| 329 | |
| 330 | fn(..., Error ** |
| 331 | - ____ |
| 332 | + errp |
| 333 | , ...) |
| 334 | { |
| 335 | ... |
| 336 | } |