errp-guard.cocci (8699B)
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 }