How to: find your first good contribution | KBlog

Finding a first issue can be easier said than done - don’t forget that you’re<br>“competing” with hundreds of other people around the world, so sometimes when<br>you send a patch fixing an issue, the maintainer will then point out that<br>somebody beat you to the punch and a fix has already been applied (this is the<br>reason why you should rebase your development tree regularly!). Nevertheless, a<br>lot of issues which need desperate fixes are sitting in Linus’ tree and nobody<br>is trying to fix them. I’ll be using examples from the IIO subsystem in this<br>post, however the things described here can be applied to other subsystems.<br>Issues will be sorted from easy to “hard”.

Typos

Probably the simplest change you can do. People make mistakes - sometimes you<br>make an extra keystroke and nobody notices during the review process. Typos in<br>the actual code are very rare (but they can happen), usually they appear in<br>comments. These could be simple grammatical errors, additional characters or<br>maybe invalid information, such as an incorrect ID value that was misread from a<br>datasheet. It’s an okay starting point - it makes the code look more<br>professional (and correct), nevertheless keep in mind that this is low hanging<br>fruit - a lot of people send a big patch series fixing typos in entire<br>subsystems, and maintainers definitely prefer these as opposed to a single patch<br>fixing “Get’s” to “Gets”.

Style issues

Okay, this varies from maintainer to maintainer. The Linux kernel has a defined<br>coding style [1]. Depending on the state of the subsystem, some maintainers choose<br>not to accept style changes as other more serious issues need fixing first<br>(a good example of this is staging/media/atomisp/, which is dedicated to<br>adding support for Intel Atom based cameras and is riddled with severe issues).<br>Subsystems that are more mature will usually want to have their codebase adhere<br>to the kernel coding style (e.g. IIO accepts style changes).

Examples of bad code style:

Multi-line function parameters that aren’t aligned with the opening parenthesis

Lines exceeding the 80 (sometimes 100) character count

Using if (foo == NULL) instead of if (!foo)

Trailing whitespace

Excessive braces around single statements

Of course, these issues can sometimes be painful to spot, this is why the kernel<br>community made a handy Perl script that finds these issues for you. To use it,<br>run the following command in the root of your kernel directory:

scripts/checkpatch.pl -f path/to/file

An example output can look like the following:

ERROR: space required after that ',' (ctx:VxV)<br>#32: FILE: drivers/iio/light/al3010.c:32:<br>+#define AL3010_GAIN_MASK GENMASK(6,4)

total: 1 errors, 0 warnings, 247 lines checked

These are great changes that improve the overall readability and maintenance<br>of the code.

Compiler warnings and Sparse

Ideally, the compiler shouldn’t produce any warnings when running; nevertheless,<br>some mistakes slip through the cracks. Running make with the W=1 flag will<br>make the compiler warn on all issues. Another good flag is C=1. This invokes<br>the Sparse static analyzer (separate install needed). Using this tool can reveal<br>more serious issues like use-after-frees and incorrect types in assignment.

These are also easy fixes, the compiler and Sparse will tell you (usually)<br>exactly what’s wrong and sometimes even how to fix it.

Include What You Use

This is a pretty good principle when programming in general - don’t add unused<br>#include headers in your files! Not only it makes your list of headers<br>messier, it also worsens the build time of your kernel/module/etc.! Originally,<br>the kernel used a header for including fundamental building<br>blocks, but over time developers started including this header everywhere<br>instead of including the actual header of the things they use. This creates a<br>not-so-fun side effect - if an unrelated header in kernel.h is touched and<br>your driver includes kernel.h, you will have to recompile the header, even if<br>it’s not used in your driver, making your build time longer.

If you’re unsure about which headers to remove and which to add, there is a<br>handy tool for this from Google called “iwyu-tool”. It’s a bit harder to use<br>and it requires LLVM, but this is luckily a place where LLMs shine - they’re<br>good at configuring this stuff. Additionally, the tool generates a lot of<br>unrelated noise; this is prevented by the use of a mapping file. IIO does have<br>a mapping file that can be used [3].

Replacing sprintf/snprintf with sysfs_emit()

Previously, if you wanted to pass a string to display in userspace via a _show() function, sprintf() or snprintf() were used to write text into the output buffer. These functions, however, aren’t protected against page buffer overflows (PAGE_SIZE). On the other hand, sysfs_emit() is safe, purpose-built for sysfs _show() callbacks, and its usage is recommended in the kernel documentation.

The diff for this change is fairly simple:

static ssize_t...