01.10.2019 19:22, Eric Blake wrote: > On 10/1/19 10:52 AM, Vladimir Sementsov-Ogievskiy wrote: >> error_append_hint and error_prepend will not work, if errp == >> &fatal_error, as program will exit before error_append_hint or >> error_prepend call. Fix this by use of special macro >> ERRP_AUTO_PROPAGATE. > > This patch doesn't actually fix any code, but adds the script to enable > automating the fixing of the code in subsequent patches. Tweaking the commit > message to make that point clear might be helpful.
Hmm, so, maybe, switch "Fix this by use ..." to To fix code with help of this script do spatch --sp-file scripts/coccinelle/fix-error-add-info.cocci FILE_TO_FIX > > >> scripts/coccinelle/fix-error-add-info.cocci | 28 +++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 scripts/coccinelle/fix-error-add-info.cocci >> >> diff --git a/scripts/coccinelle/fix-error-add-info.cocci >> b/scripts/coccinelle/fix-error-add-info.cocci >> new file mode 100644 >> index 0000000000..34fa3be720 >> --- /dev/null >> +++ b/scripts/coccinelle/fix-error-add-info.cocci >> @@ -0,0 +1,28 @@ >> +@rule0@ >> +// Add invocation to errp-functions >> +identifier fn; >> +@@ >> + >> + fn(..., Error **errp, ...) >> + { >> ++ ERRP_AUTO_PROPAGATE(); >> + <+... >> +( >> + error_append_hint(errp, ...); >> +| >> + error_prepend(errp, ...); >> +) > > So, for now, you aren't addressing any *errp usage, or any potential cleanups > of error_propagate. But that's okay; your cover letter did call out that you > were only addressing 1 part out of 3 potential uses just to get some motion, > based on the size of the effort. > >> + ...+> >> + } >> + >> +@@ >> +// Drop doubled invocation >> +identifier rule0.fn; >> +@@ >> + >> + fn(...) >> +{ >> + ERRP_AUTO_PROPAGATE(); >> +- ERRP_AUTO_PROPAGATE(); >> + ... >> +} >> > > This looks idempotent once a file is patched, and safe to rerun as many times > in the future as needed. I'm still hoping we can find a way to make > scripts/checkpatch.pl also do a sanity check, but as it's harder to parse C > in perl than in Coccinelle, I can live with just the .cocci script in-tree as > long as we remember to manually run it periodically. scripts/checkpatch.pl is so unfriendly.. And to run coccinelle scripts we need working directory, not just patch files. I imagine the following: move scripts/checkpatch.pl to scripts/checkpatch dir Create scripts/checkpatch.py (or python/checkpatch.py, I don't know what is the idea of python/ dir), which will work only on commits or on patch files with specified base (master by default), it will create temporary worktree, checkout corresponding commit and then run scripts from scripts/checkpatch/, at least it would be checkpatch.pl and coccinelle.sh (which will run some coccinelle scripts from coccinelle dir).. > > Reviewed-by: Eric Blake <ebl...@redhat.com> > -- Best regards, Vladimir