Proper Linux Kernel Coding Style

As more people and companies start to write Linux kernel code, understanding acceptable kernel programming coding style and conventions is very important. This article starts out by explaining the written kernel coding style, as documented in the kernel's Documentation/CodingStyle file, and then moves on to introduce some of the “unwritten” kernel coding style rules.

Why are there kernel programming style rules in the first place? Why not just let authors code in whatever style they want, and let everyone live with it? After all, code formatting does not affect memory use, execution speed or anything else a normal user of the kernel would see. The reason for rules can be summed up with this passage from Elliot Soloway and Kate Ehrlich's article (see Resources):

There have been a number of other studies about how a large body of code written in a common style directly affects how easy it is to understand the code, review it and revise it quickly.

Because the number of developers that look at the Linux kernel code is very large, it is in the best interest for the project to have a consistent style guideline. This allows the code to be easier to understand by both someone reading it for the first time and for someone revisiting his or her old code a number of months later. It also allows someone else to read easily, understand and potentially fix and enhance your code, which is one of the greatest strengths of open-source code.

So now that we have an understanding of why there should be rules, what are they? Linus Torvalds and other kernel programmers have written a short document that details some of the kernel programming rules. This document is located in the Documentation/CodingStyle file in the kernel source tree. It is required reading for anyone who wants to contribute to the Linux kernel. The following is a summary of these rules.

All tabs are eight characters and will be the <TAB> character. This makes locating where different blocks of code start and end easier. If you find that your code is indented too deeply, with more than three levels of indentation that cause the code to shift to the right of the screen too far, then you should fix this.

The original authors of UNIX placed their braces with the opening brace last on the line, and the closing brace first on the line:

if (x is true) {
        we do y
}

Therefore, the Linux kernel uses this style. The exceptions to this rule are functions, which have the opening brace at the beginning of the line:

int function(int x)
{
        body of function
}

For good examples of proper indentation and brace style, look at any of the fs/*.c files or anything in the kernel/*.c files. Generally, most of the kernel is in the proper indentation and brace style, but there are some notable exceptions. The code in fs/devfs/*.c and drivers/scsi/qla1280.* are good examples of how not to do indentation and braces.

A script can be used to run the indent program if you have to convert a large amount of code to the correct format. This file is located at scripts/Lindent in the kernel source tree.

Your variables and functions should be descriptive and concise. Don't use long flowery names like, CommandAllocationGroupSize or DAC960_V1_EnableMemoryMailboxInterface() but rather, cmd_group_size or enable_mem_mailbox(). Mixed-case names are frowned upon, and encoding the type of the variable or function in the name (like “Hungarian notation”) is forbidden.

Global variables should be used only if they are absolutely necessary. Local variables should be short and to the point. Valid local loop variable names include “i” and “j”, while “loop_counter” would be too verbose; “tmp” is allowed for any short-lived temporary variable.

Again, good examples of proper names can be found in fs/*.c. A lot of driver code has bad variable names, as it has been ported from other operating systems. Examples of how not to name functions and variables include drivers/block/DAC960.* and drivers/scsi/cpqfs*.

Functions should do only one thing and do it well. They should be short and hopefully contain only one or two screens of text. If you have a function that does a lot of small things for different cases, it is acceptable to have a longer function. But if you have a long, complex function, you have a problem.

In addition, a large number of local variables within a function is a measure of how complex the function is. If the number of local variables exceeds ten, then there is something wrong.

Again, there are a lot of good examples of nice-sized functions in the fs/*.c and other kernel core code. Some bad examples of functions can be found in drivers/hotplug/ibmphp_res.c (where one function is 370 lines long) or drivers/usb/usb-uhci.c (where one function has 18 local variables).

Comments are good to have, but they have to be useful. Bad comments explain how the code works, who wrote a specific function on a specific date or other such useless things. Good comments explain what the file or function does and why it does it. They should also be at the beginning of the function and not necessarily embedded within the function (you are writing small functions, right?).

Now there is also a standard format for function comments. It is a variant of the documentation method used by the GNOME Project for its code. If you write your function comments in this style, the information in them can be extracted by a tool and made into standalone documentation. This can be seen by running make psdocs or make htmldocs on the kernel tree to generate a kernel-api.ps or kernel-api.html file containing all of the public interfaces to the different kernel subsystems.

This style is documented in the file Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc. The basic format is as follows:

/**
 * function_name(:)? (-
(* @parameterx: (
(* a blank line)?
 * (Description:)? (
 * (section header: (
(*)?*/

The short function description cannot run multiple lines, but the other descriptions can (and they can contain blank lines). All further descriptive text can contain the following markups:

So a simple example of a function comment with a single argument would be:

/**
 * my_function - does my stuff
 * @my_arg: my argument
 *
 * Does my stuff explained.
 **/

Comments can and should be written for structures, unions and enums. The format for them is much like the function format:

/**
 * struct my_struct - short description
 * @a: first member
 * @b: second member
 *
 * Longer description
 */
struct my_struct {
    int a;
    int b;
};

A chapter on data structures showed up in the 2.4.10-pre7 kernel. It goes into how every data structure that can exist outside of a single-threaded environment needs to implement reference counting in order to handle the memory management issues of the structure properly. If you add reference counting to your structure, you can avoid a lot of nasty locking issues and race conditions. Multiple threads can access the same structure without having to worry that a different thread will free the data from under it.

The last sentence in this chapter is required reading by any kernel developer: “Remember: if another thread can find your data structure, and you don't have a reference count on it, you almost certainly have a bug.”

A good example of why reference counting is necessary can be found in the USB data structure, “struct urb”. This structure is created by a USB device driver, filled with data, sent to a USB host controller where it will be processed and eventually sent out over the wire. When the host controller is finished with the urb, the original device driver will be notified. While a host controller driver is processing the urb, the original driver can try to cancel the urb or even free it. This led to numerous bugs in the core USB subsystem and different drivers. It also created long, detailed arguments on the linux-usb-devel mailing list about when in the life span of a urb it was allowed to be touched by either driver. In the 2.5 kernel series, struct urb had a reference count added to it, and the USB core and USB host controller drivers had a small amount of code added to handle the reference count properly. Now, whenever a driver wants to use the urb, a reference count is incremented. When it is finished with it, the reference count is decremented. If this was the last user, the memory is then freed and the urb disappears. This allows the USB device drivers to simplify their urb-handling logic vastly, fixes a lot of different race condition bugs and quiets all arguments about the topic.

If you follow the above set of rules, your code will look like good Linux kernel code. However, quite a few unwritten rules and style guidelines exist that good kernel code follows. Here are some of them.

A wide variety of well-designed, well-documented and well-debugged function and data structures can be found within the kernel. Please take advantage of them, and do not reinvent your own version of them just because you did not write them. Among the most common of these are the string functions, the byte order functions and the linked list data structure and functions.

In the file include/linux/string.h, a number of common string handling functions are defined. These include strpbrk, strtok, strsep, strspn, strcpy, strncpy, strcat, strncat, strcmp, strncmp, strnicmp, strchr, strrchr, strstr, strlen, strnlen, memset, memcpy, memove, memscan, memcmp and memchr. And a number of “simple” string functions are defined in the file, include/linux/kernel.h: simple_strtoul, simple_strtol, simple_strtoull and simple_strtoll. So, if you need any type of string functionality in your kernel code, please use the built-in functions. Do not try to rewrite the existing functions accidentally.

Do not rewrite code to switch data between different endian representations. The file include/asm/byteorder.h (asm will point to the proper subdirectory, depending on your processor architecture) will bring in a wide range of functions that allow you to do automatic conversions, regardless of the endian format of your processor or your data.

If you need to create a linked list of any kind of data structure, use the code that is in include/linux/list.h. It contains a structure, struct list_head, that should be included within the structure of which you want to create a list. Then you will be able to add, remove or iterate over a list of data structures easily, without having to write new code.

Some good examples of code that uses the list structure can be found in drivers/hotplug/pci_hotplug_core.c and drivers/ieee1394/nodemgr.c. Some code in the kernel that should be using the list structure can be found in the ATM core, within the struct atm_vcc data structure. Because the ATM code did not use struct list_head, every ATM driver needs to walk the lists of data structures by hand, duplicating a lot of code.

typedef should not be used in naming any of your structures. Most main kernel structures have no typedef. Though this would shorten their usage, it obscures code. For example, the core kernel structures struct inode, struct dentry, struct file, struct buffer_head, struct user and struct task_struct have no typedef.

Using a typedef only hides the real type of a variable. There are records of some kernel code using typedefs nested up to four layers deep, preventing the programmer from easily telling what type of variable they are actually using. If the programmer does not realize the size of the structure it can cause very large structures to be accidentally declared on the stack or to be returned from functions.

typedef also can be used as a crutch to keep from typing long structure definitions. If this is the case, the structure names should be made shorter, according to the above-listed naming rules.

Never define a typedef to signify a pointer to a structure, as in the following example:

typedef struct foo {
        int bar;
        int baz;
} foo_t, *pfoo_t;

This hides the true type of the variable and uses the name of the variable type to define what it is (see the previous comment about Hungarian notation).

Some examples of where typedefs are used badly are in the include/raid/md*.h files, where every structure has a typedef assigned to it, and in the drivers/acpi/include/*.h files, where a lot of the structures do not even have a name assigned to them, only a typedef.

The only place that using typedef is acceptable is in declaring function prototypes. These can be difficult to type out every time, so declaring a typedef for these is nice to do, for example, the bh_end_io_t typedef that is used as a parameter in the init_buffer() call. This is defined in include/fs.h as:

typedef void (bh_end_io_t)
    (struct buffer_head *bh, int uptodate);

With the wide number of different processors, different configuration options and variations of the same base hardware types that Linux runs on, it is easy to start having a lot of #ifdef statements in your code. This is not the proper thing to do. Instead, place the #ifdef in a header file, and provide empty inline functions if the code is not to be included.

As an example, consider the following code in drivers/usb/hid-core.c:

static void hid_process_event (struct hid_device *hid,
                               struct hid_field *field,
                               struct hid_usage *usage,
                               __s32 value)
{
   hid_dump_input(usage, value);
   if (hid->claimed & HID_CLAIMED_INPUT)
         hidinput_hid_event(hid, field, usage, value);
#ifdef CONFIG_USB_HIDDEV
   if (hid->claimed & HID_CLAIMED_HIDDEV)
         hiddev_hid_event(hid, usage->hid, value);
#endif
}

Here the author does not want to call hiddev_hid_event() if a specific configuration option is not enabled. This is because that function will not even be present if the configuration option is not enabled.

To remove this #ifdef, the following change was made to include/linux/hiddev.h:

#ifdef CONFIG_USB_HIDDEV
   extern void hiddev_hid_event (struct hid_device *,
                                 unsigned int usage,
                                 int value);
#else
   static inline void
   hiddev_hid_event (struct hid_device
*hid,
                     unsigned int usage,
                     int value) { }
#endif

Then drivers/usb/hid-core.c was changed to:

static void hid_process_event
                           (struct hid_device *hid,
                            struct hid_field *field,
                            struct hid_usage *usage,
                            __s32 value)
{
   hid_dump_input(usage, value);
   if (hid->claimed & HID_CLAIMED_INPUT)
         hidinput_hid_event(hid, field, usage, value);
   if (hid->claimed & HID_CLAIMED_HIDDEV)
         hiddev_hid_event(hid, usage->hid, value);
}

The Linux kernel consists of a large amount of source code, contributed by hundreds of developers over many years. Because the majority of this code follows some simple and basic style and formatting rules, the ability for people to understand new code quickly has been greatly enhanced. If you want to contribute to this code, please read the CodingStyle guidelines and follow them in your patches and new code. The other unwritten rules can be just as important as the written ones when you are trying to convince people to accept your contributions, and they should be followed just as closely.

This article was based upon a paper and presentation that was created for the 2002 Ottawa Linux Symposium.

Resources