Coccinelle challenge problems from OPW round 8.

Coccinelle challenge problem 1

The following semantic patch introduces the use of the managed version of kzalloc in some very constrained cases:

@platform@
identifier p, probefn, removefn;
@@
struct platform_driver p = {
  .probe = probefn,
  .remove = removefn,
};

@other_things depends on platform@
@@

( iio_device_alloc(...)
| iio_trigger_alloc(...)
| iio_device_register(...)
| request_region(...)
| request_mem_region(...)
| request_irq(...)
| dma_alloc_coherent(...)
| dma_alloc_noncoherent(...)
| dma_declare_coherent_memory(...)
| dma_pool_create(...)
| pci_enable_device(...)
| pci_pin_device(...)
| ioport_map(...)
| ioremap(...)
| ioremap_nocache(...)
| pcim_iomap(...)
| pcim_iomap_table(...)
| pcim_iomap_regions(...)
| regulator_get(...)
| regulator_bulk_get(...)
| regulator_register(...)
| clk_get(...)
| pinctrl_get(...)
| pwm_get(...)
| usb_get_phy(...)
| acpi_dma_controller_register(...)
| spi_register_master(...)
| gpio_request(...)
| gpio_request_one(...)
)

@prb depends on !other_things@
identifier platform.probefn, pdev;
expression e, e1, e2;
@@
probefn(struct platform_device *pdev, ...) {
  <+...
- e = kzalloc(e1, e2)
+ e = devm_kzalloc(&pdev->dev, e1, e2)
  ...
?-kfree(e);
  ...+>
}

@rem depends on prb@
identifier platform.removefn;
expression e;
@@
removefn(...) {
  <...
- kfree(e);
  ...>
}

Do the following:

1. Read about the use of devm functions in Documentation/driver-model/devres.txt. Any recent version is fine.
2. Download and install Coccinelle. If you are using Linux, it should be available in your package manager. Any recent version is fine.
3. Download staging-next
4. Save the above semantic patch in a file kzalloc.cocci

5. Run Coccinelle on kzalloc.cocci and staging-next, ie spatch --sp-file kzalloc.cocci --no-includes --dir {your staging-next path} > kzalloc.out. This may take some time. Don't worry if there are some error messages.

6. Study the results in kzalloc.out carefully and submit a patch based on one case where you think that the transformation was done properly.

This semantic patch is not very robust, in that there is no guarantee that the kfree that is removed in the remove function is freeing the data that was kzalloced in the probe function. Some things to watch out for are as follows:

1. Does the file already use devm functions? If so, any kzalloc and kfree that this semantic patch finds are probably still there for a reason.
2. Does the file contain calls to kfree that are not in the probe and remove functions. If so, could they affect the kzalloced data? If so, more thought may be required as to whether the kfrees are needed, and if they should be transformed to devm_kfree for early release of the devm allocated data.
3. Does the transformation affect both a probe function and a remove function, or only a probe function? If there is no transformation to a remove function, is the kfree somewhere else? Is there a reason why the allocated data should not be freed? Again, more thought may be needed.

Coccinelle challenge problem 2

One of the checkpatch errors is "do not use assignment in if condition". And example of this is the following code, from drivers/staging/cxt1e1/sbeproc.c:

        if (!(bip = OS_kmalloc(sizeof(struct sbe_brd_info))))
                return -ENOMEM;

Write a semantic patch to find and fix such cases, eg producing the following patch for the above example:

-       if (!(bip = OS_kmalloc(sizeof(struct sbe_brd_info))))
+       bip = OS_kmalloc(sizeof(struct sbe_brd_info));
+       if (!bip)
                return -ENOMEM;

Apply your semantic patch to the Linux staging directory and submit patches based on your results. Take case that your result is as you would like it to look if you were making the transformation by hand. If it is not, consider whether you should extend your semantic patch in some way to improve the result. Include a concise version of the semantic patch that you have used in your commit message.

Coccinelle challenge problem 3

If a variable has value 0, then there is no point in combining it with other things with |, as for any x, 0 | x is just x. The following semantic patch finds this problem.

@@
expression x,e,e1;
statement S;
@@

if (x == 0) {
  ... when != x = e1
      when != while(...) S
      when != for(...;...;...) S
(
*  x |= e
|
*  x | e
)
  ... when any
}

1. Apply the semantic patch to the Linux kernel and make some corresponding changes by hand. Note that the result of a semantic patch that uses * is something like a patch with a - at the beginning of any line that contains a match of the starred pattern.

2. Consider how you could extend the semantic patch to fix the code rather than just finding possible occurrences of the problem. Hint: it may work best to change the first "..." to <... and to change the "... when any" to just ...>.

Coccinelle challenge problem 4

The Linux kernel provides a number of specific printing functions, eg for net devices and for devices in general. The goal of this challenge problem is to use Coccinelle to convert the use of the generic functions, eg pr_debug, to the net device ones, eg netdev_dbg. The challenge in doing this is to find a net_device typed pointer that can be used as the first argument of the netdev printing function. Often such a pointer is accessible from on of the parameters of the function enclosing the call.

Your semantic patch should consist of three rules:

1. To find a call to eg pr_debug and the structures in the parameter list of its enclosing function.
2. To find the net_device field in one of these structures.
3. To transform the call to add the reference to the field.

An implementation of the first rule is shown below. It considers only pr_debug:

@r exists@
identifier f,s,i;
position p;
@@

f(...,struct s *i,...) {
  <+...
  pr_debug@p(...)
  ...+>
}

Implement the other two rules for the pr_debug case. Consider how to extend the semantic patch to other pr_ functions.

Hint: Give spatch the additional arguments --all-includes and -I path_to_your_kernel/include. This will allow it to find more structure type definitions from header files. Using the latest version of Coccinelle, from the Coccinelle web page (http://coccinelle.lip6.fr/distrib/coccinelle-1.0.0-rc20.tgz) may give better performance.

Coccinelle challenge problem 5

A number of people have proposed patches that fix spacing issues between certain kinds of tokens, such as the need for a space after an if or an extra space after the name of a function in a function call. Making such small changes is probably pretty easy to do by hand, but using Coccinelle for it permits to illustrate some extra features of Coccinelle.

The following semantic patch finds cases where there is not exactly one space between if and the following (, and reconstructs the if when the problem occurs.

@r@
position p1,p2;
statement S1,S2;
@@

(
if@p1 (@p2 ...) S1 else S2
|
if@p1 (@p2 ...) S1
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

l1 = int (p1[0].line)
l2 = int (p2[0].line)
c1 = int (p1[0].column_end)
c2 = int (p2[0].column)
if (l1 == l2 and c1 + 1 == c2):
  cocci.include_match(False)

@@
position r.p1;
statement S1,S2;
@@

(
- if@p1 (
+ if (
  ...) S1 else S2
|
- if@p1 (
+ if (
  ...) S1
)

The semantic patch consists of three rules.

1. The rule r simply matches every if, with either a then and an else branch, or only a then branch. It uses one "position variable", p1, to record the position of the if token, and another, p2, to record the position of the open parenthesis.
2. The second rule contains python code that checks whether there is exactly one space between the if and the open parenthesis. If this is the case, the pair of values of p1 and p2 are discarded, as though they had never matched.
3. The third rule removes the if and the open parenthesis and then puts it back, taking advantage of the fact that the Coccinelle pretty printer puts a space between if and an open parenthesis.

The result of this semantic patch is not always satisfactory, in that it may cause the introduction of extra braces. Coccinelle is not able to realize that this transformation amounts to just replacing a single statement by a single statement, and thus if the if is an immediate child of another if, while, etc, with no surrounding braces, it adds some. One way not to be bothered by these cases (while introducing false negatives, ie overlooked cases) would be to put a function call or assignment pattern just before the if in the first rule.

This semantic patch is rather slow. Consider why that would be the case.

Extend the semantic patch to address other cases, such as no space between ) and { in an if.

Contact info

Email: <Julia.Lawall AT lip6 DOT fr>

My IRC handle is jlawall.

Questions about using Coccinelle should go to the Coccinelle mailing list: <cocci AT systeme DOT lip6 DOT fr>

---

CategoryHomepage