Style and Checks

Everything in this chapter will be run against your patch whether you run it or not. The only question is whether you find out privately, in thirty seconds, or publicly, three days later, in a mail from a robot with a hundred people Cc'd.

Run them first.

The Pre-Send Checklist

Do this from top to bottom before every git send-email. It takes about ten minutes for a small patch and it is the difference between a clean first impression and a round trip.

cd ~/kernel/linux

# 1. Style, on your commits (not on files you did not touch).
./scripts/checkpatch.pl --strict -g HEAD~1..HEAD

# 2. It builds, for your config.
make O=../build -j"$(nproc)"

# 3. It builds for configs you do not use. This is what the bots do.
make O=../build-all allmodconfig && make O=../build-all -j"$(nproc)"

# 4. It builds with a different compiler.
make O=../build-clang LLVM=1 defconfig && make O=../build-clang LLVM=1 -j"$(nproc)"

# 5. Static analysis on the files you touched.
make O=../build C=2 W=1 <your/files.o>
make O=../build coccicheck MODE=report M=<your/dir>

# 6. Every commit in the series builds.
git rebase --exec 'make -j$(nproc) O=../build' <base>

# 7. It actually works, under the paranoid kernel.
PROFILE=paranoid ~/kernel-labs/scripts/build-kernel.sh
~/kernel-labs/scripts/run-qemu.sh        # and run your test

# 8. The formalities.
grep -c '^Signed-off-by:' *.patch        # one per patch
grep '^base-commit:' *.patch             # from --base=auto
./scripts/get_maintainer.pl *.patch      # the recipients

Documentation/process/submit-checklist.rst is the in-tree version of this list, it is maintained, and it is shorter than this chapter:

$EDITOR ~/kernel/linux/Documentation/process/submit-checklist.rst

checkpatch.pl

./scripts/checkpatch.pl --strict -f drivers/foo/bar.c   # a whole file
./scripts/checkpatch.pl --strict *.patch                # a produced patch
./scripts/checkpatch.pl --strict -g HEAD~3..            # your commits
./scripts/checkpatch.pl --strict --show-types -f x.c    # so you can name a warning

Two rules that matter more than the tool:

Only check what you touched. checkpatch -f on an old file produces hundreds of findings about code that is not yours. Fixing them turns a one-line patch into an unreviewable mess. Use -g on your commits.

Do not blindly obey it. checkpatch is a set of heuristics and it is wrong often enough to matter — it flags legitimate long lines in strings, misidentifies some macros, and has opinions about comment style that differ from net/'s actual convention. Understand each finding; fix the real ones; be able to defend the ones you leave.

Warning: A patch whose entire content is checkpatch fixes to code you have not read is a classic way to get a bad first impression. It creates review load, risks breaking something you do not understand, and reads as chasing a contribution count rather than fixing anything. If you want to fix style, fix it in code you are already changing for a real reason.

Coding Style, in Practice

$EDITOR ~/kernel/linux/Documentation/process/coding-style.rst

Short, opinionated, and worth reading in full once. The parts people get wrong:

There is a .clang-format in the tree. Use it on your new code only:

git diff -U0 --cached | ./scripts/clang-format-diff.py -p1 -i

Never reformat surrounding code in a patch that does something else. A style change mixed into a functional change is one of the fastest ways to be asked to split.

The Static Checkers

# sparse only complains about the files being compiled, so touch yours:
touch drivers/foo/bar.c && make O=../build C=1 drivers/foo/

# Coccinelle can also FIX what it finds:
make coccicheck MODE=patch COCCI=scripts/coccinelle/free/kfree.cocci M=drivers/foo/

sparse is the one that earns its keep for driver work: a __user pointer used directly or a __le32 compared as native are invisible to GCC, compile fine, and are wrong on half the machines in the world.

Building the Way the Bots Build

Your config is one point in a space of thousands of CONFIG_ symbols. "It builds for me" carries almost no information.

# The two highest-yield, in one go:
make O=../b-all  allmodconfig  && make O=../b-all  -j"$(nproc)" 2>&1 | tee /tmp/all.log
make O=../b-arm  ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- defconfig \
  && make O=../b-arm ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j"$(nproc)"
grep -E "warning|error" /tmp/all.log | grep -F "$(git diff --name-only HEAD~1 | head -1)"

The Robots

Four systems will look at your patch without being asked. Knowing what each does removes the surprise and, more importantly, tells you what they cannot catch.

$EDITOR ~/kernel/linux/Documentation/dev-tools/kselftest.rst
$EDITOR ~/kernel/linux/Documentation/dev-tools/syzbot.rst

Note: A 0-day report is not a rejection and not an insult. It is a free, thorough CI run that nobody paid for, and the correct response is a quick "thanks, fixed in v2". The reports are public and everyone gets them; the only thing that looks bad is ignoring one.

What the robots cannot catch, and therefore what human review is for: whether your change is a good idea, whether the locking is right, whether the ABI is extensible, and whether it belongs in this subsystem at all.

Reading Exercise

# 1. Run checkpatch --strict -f on a file in your subsystem you did NOT write.
#    How many findings? How many are real? This calibrates how much to trust it.
./scripts/checkpatch.pl --strict --show-types -f <some file> | tail -30

# 2. Find a 0-day report on lore. Search for "kernel test robot" plus your
#    subsystem's list name. Read the report and then the author's reply.

# 3. Find a syzbot report with a reproducer, and read the whole thread
#    including the fix. Note how much of the analysis the bot did.

# 4. Run make W=1 on one directory and count how many warnings pre-date you.
make O=../build W=1 drivers/<something>/ 2>&1 | grep -c warning

Common Mistakes and Their Symptoms

Validation / Self-check

1. Give the eight steps of the pre-send checklist, and say which one you would drop under time pressure and why.
2. What are checkpatch's three severity levels, and how does --strict change what you see?
3. Why run checkpatch -g on commits rather than -f on files?
4. Give two cases where checkpatch is wrong, and say how you would defend leaving a finding unfixed.
5. Why is a patch consisting only of checkpatch fixes a bad first contribution?
6. What does sparse catch that GCC does not? Give two concrete classes.
7. What is the difference between make C=1 and C=2, and when do you want each?
8. Why does allmodconfig break code that builds cleanly for you?
9. Name four automated systems that will examine your patch, and what each is good at.
10. What is the correct response to a 0-day report, and what does ignoring one signal?
11. Name four things no robot can check, which is therefore what human review is for.
12. Which comment style does net/ use, and how would you find that out without being told?

Next: Lab 7 — Your First Patch. Everything above was preparation; this is the one you actually send.