Claudio Fontana <[email protected]> writes: > Signed-off-by: Claudio Fontana <[email protected]> > --- > scripts/checkpatch.pl | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > We could do something similar to MAINTAINERS for trace-events, > ie warning about files added, moved, deleted if we don't see > an update to a trace-events file.
I like the idea, but... > To make it more solid it would be better to check the > actual file contents for #include "trace.h" or "trace-root.h", > but I guess this is not possible/practice from checkpatch? ... I'm also concerned about false positives. > If we could only warn for files moved that actually include > trace.h or trace-root.h, we could avoid a lot of false positives. The existing MAINTAINERS check warns even when an existing pattern covers the new file, e.g. when I create or rename a file scripts/qapi/* or qapi/*.json. The former is definitely a false positive, and mildly annoying. The latter, however, could be a true positive: even though the new file is covered by the "QAPI Schema" section, *additional* coverage by some other section may be called for, just like qapi/machine.json is additionally covered by "Machine core". So, even if checkpatch.pl looked at more than just the patch, suppressing false positives would involve guesswork. The new trace-events check is simpler: it's *always* a false positive when the file doesn't include trace.h or trace-root.h. Complication: it could include via some header. I figure that's rare enough to be ignored. Howver, checkpatch.pl checks *patches* by design[*]. It doesn't read the patched files to get more context, or additional files. Perhaps it's simply the wrong place both for the MAINTAINERS check and the trace-events check. We put the MAINTAINERS check there, because it exists and developers run it. "make check-source" would be another option, except it doesn't exist. CI would be yet another option, but we should be careful about what to check only in CI. [*] There's -f to check a source file, which checks "diff -u /dev/null $filename".
