This page lists some possible cleanups that I've seen while reading btrfs code. Instances of these possible cleanups might come and go over time as patches are applied to address them and as new code adds new instances of the problems.

Use WARN_ON()s return value

Sometimes people issue a warning based on a condition by writing:

if (condition) {
    WARN_ON(1);

This is more work than is necessary because WARN_ON() returns its condition argument specifically so that people can write:

  if (WARN_ON(condition)) {

It results in clearer source code and the resulting kernel warning message will output the source code line that contains the condition, not just the WARN_ON(1) call.

Instances of this can be found with grep. For example:

 $ grep -B 1 'WARN_ON(1)' fs/btrfs/*.c | head
fs/btrfs/backref.c-             if (!level) {
fs/btrfs/backref.c:                     WARN_ON(1);

Replacing multiple atomic_inc or _dec with _add or _sub

Sometimes people write multiple atomic increments immediately following each other, like:

  atomic_inc(&something);
  atomic_inc(&something);

This can be done with both less source code and fewer compiled instructions with:

  atomic_add(&something, 2);

The same applies to atomic_dec() and atomic_sub()

replace kmalloc(size * nr, ) with kmalloc_array(nr, size)

Instances of the pattern:

  kmalloc(sizeof(something) * nr, ...)

should be replaced with

  kmalloc_array(nr, sizeof(something), ...)

kmalloc_array() is preferred because it can check that the calculation doesn't wrap and won't return a smaller allocation.

add helper for free_root_pointers()

disk-io.c:free_root_pointers() contains many copies of the following code:

        if (info->dev_root) {
                free_extent_buffer(info->dev_root->node);
                free_extent_buffer(info->dev_root->commit_root);
                info->dev_root->node = NULL;
                info->dev_root->commit_root = NULL;
        }

For each root pointer. A helper function should be created which is called for each of the root pointers.

remove useless variables

Sometimes tiny functions add a useless variable for returning the result of another function. The pattern is:

int function(arguments)
{
    int ret;
    ret = another_function(arguments);
    return ret;
}

This serves no purpose. At the very least, the function should just return the result of the called function:

int function(arguments)
{
    return another_function(arguments);
}

Implement a core btrfs 'find item' interface

This one is more advanced, but I thought I'd throw it out there.

There are many btrfs functions that manually search the tree for an item. They all reimplement the same mechanism and differ in the conditions that they use to find the item.

__inode_info() is one such example. It wants to set the caller's path to point to the first item in the tree after its specified objectid, type, and offset for which objectid and type match the input.

So a core function should be written that does this and __inode_info() should call it.

From there we can start identifying other callers that could use the core item searching function.