|
Size: 11853
Comment:
|
Size: 7606
Comment:
|
| Deletions are marked like this. | Additions are marked like this. |
| Line 8: | Line 8: |
| Please write to me directly if you would like to apply to the Coccinelle OPW project. | Please write to me directly if you would like to apply to the Coccinelle Outreachy project. |
| Line 12: | Line 12: |
| These challenge problems are listed roughly in order of increasing difficulty. In particular, problems 1-3 go together, and problems 4-5 go together. It is not obligatory to do all of them. You may find other things that can be done with Coccinelle. Source of inspiration may be the results of checkpatch and patches that have been applied to the kernel in the past. Any kind of problem that occurs over and over might be amenable to being solved with Coccinelle. | It would be a good idea to start with the first challenge problem, to check that you know how to use the tool properly. The remaining challenge problems can be done in any order. It is not obligatory to do all of them. You may find other things that can be done with Coccinelle. Sources of inspiration may be the results of checkpatch and patches that have been applied to the kernel in the past. Any kind of problem that occurs over and over might be amenable to being solved with Coccinelle. |
| Line 14: | Line 14: |
| These challenge problems may apply to many files in the kernel. Pick a few files, and send patches for those. Once they have been accepted, consider moving on to another challenge problem. You will get a better understanding of Coccinelle if you use it for many different thing than if you use it do one thing over and over. | These challenge problems may apply to many files in the kernel. Pick a few files, and send patches for those. Once they have been accepted, consider moving on to another challenge problem. You will get a better understanding of Coccinelle if you use it for many different things than if you use it do one thing over and over. There are many examples of uses of Coccinelle, in previous patches, in the kernel source tree in the scripts/coccinelle directory, and at [https://github.com/coccinelle/coccinellery coccinellery]. If you use a script that is already in the Linux kernel, you don't need to include the script in your commit log, but rather something like Generate-by: sripts/coccinelle/misc/badty.cocci |
| Line 22: | Line 24: |
| Consider the following function, from drivers/staging/vt6656/rf.c: | Consider the following function, from drivers/staging/most/hdm-dim2/dim2_sysfs.c |
| Line 25: | Line 27: |
| int vnt_rf_setpower(struct vnt_private *priv, u32 rate, u32 channel) | static ssize_t bus_kobj_attr_store(struct kobject *kobj, struct attribute *attr, const char *buf, size_t count) |
| Line 27: | Line 30: |
| int ret = true; u8 power = priv->cck_pwr; |
ssize_t ret; struct medialb_bus *bus = container_of(kobj, struct medialb_bus, kobj_group); struct bus_attr *xattr = container_of(attr, struct bus_attr, attr); |
| Line 30: | Line 35: |
| if (channel == 0) return -EINVAL; |
if (!xattr->store) return -EIO; |
| Line 33: | Line 38: |
| switch (rate) { case RATE_1M: case RATE_2M: case RATE_5M: case RATE_11M: channel--; if (channel < sizeof(priv->cck_pwr_tbl)) power = priv->cck_pwr_tbl[channel]; break; case RATE_6M: case RATE_9M: case RATE_18M: case RATE_24M: case RATE_36M: case RATE_48M: case RATE_54M: if (channel > CB_MAX_CHANNEL_24G) power = priv->ofdm_a_pwr_tbl[channel-15]; else power = priv->ofdm_pwr_tbl[channel-1]; break; } ret = vnt_rf_set_txpower(priv, power, rate); return ret; |
ret = xattr->store(bus, buf, count); return ret; |
| Line 66: | Line 46: |
| int vnt_rf_setpower(struct vnt_private *priv, u32 rate, u32 channel) | static ssize_t bus_kobj_attr_store(struct kobject *kobj, struct attribute *attr, const char *buf, size_t count) |
| Line 68: | Line 49: |
| int ret = true; u8 power = priv->cck_pwr; |
ssize_t ret; struct medialb_bus *bus = container_of(kobj, struct medialb_bus, kobj_group); struct bus_attr *xattr = container_of(attr, struct bus_attr, attr); |
| Line 71: | Line 54: |
| if (channel == 0) return -EINVAL; |
if (!xattr->store) return -EIO; |
| Line 74: | Line 57: |
| switch (rate) { ... } return vnt_rf_set_txpower(priv, power, rate); |
return xattr->store(bus, buf, count); |
| Line 86: | Line 65: |
| expression ret; identifier f; |
expression e, ret; |
| Line 92: | Line 70: |
| f(...); | e; |
| Line 99: | Line 77: |
| with, but you may need to get the most recent version, which is 1.0.0-rc21. This is available on the Coccinelle webpage (coccinelle.lip6.fr) and on github. 1. Download staging-next |
with, but you may need to get the most recent version, which is 1.0.4. This is available on the Coccinelle webpage (coccinelle.lip6.fr) and on github. 1. Download staging-testing |
| Line 102: | Line 80: |
| 1. Run Coccinelle on ret.cocci and staging-next, ie spatch --sp-file ret.cocci --no-includes --dir {your staging-next path}/drivers/staging > ret.out. This may take some time. |
1. Run Coccinelle on ret.cocci and staging-testing, ie spatch --sp-file ret.cocci --no-includes --dir {your staging-testing path}/drivers/staging > ret.out. This may take some time. |
| Line 114: | Line 91: |
| In the following function, from drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c, the variable ret is not very useful. |
Parentheses are not needed around the right hand side of an assignment, like in value = (FLASH_CMD_STATUS_REG_READ << 24);. Write a semantic patch to remove these parentheses. |
| Line 118: | Line 93: |
| {{{ static int rtw_cfg80211_monitor_if_set_mac_address(struct net_device *ndev, void *addr) { int ret = 0; DBG_8723A("%s\n", __func__); return ret; } }}} The code would be simpler as: {{{ static int rtw_cfg80211_monitor_if_set_mac_address(struct net_device *ndev, void *addr) { DBG_8723A("%s\n", __func__); return 0; } }}} The following semantic patch makes this transformation: {{{ @@ identifier ret; @@ -int ret = 0; ... when != ret when strict -return ret; +return 0; }}} The code <code>... when != ret</code> means that between the int ret = 0; and the return at the end of the function, there should be no use of ret. The code <code>when strict</code> means that this should hold on every execution path, including those that abort the function (return in the middle of a function) Test this semantic patch on the staging tree. Do you find any of the results surprising? Are the results correct? Did Coccinelle complain about anything or crash (if so, you may need to get a more recent version). Submit some patches based on your results. |
One could consider that parentheses might be useful in the case of eg rising = (dir == IIO_EV_DIR_RISING); because there could be a confusion between the different kinds of =. Extend your semantic patch using a disjunction so that it does not report on such cases. |
| Line 169: | Line 97: |
| If you look back at the code in == Coccinelle challenge problem 1, you will see that actually the only use of ret is the one at the end of the function, and the semantic patch gets rid of it. This suggests that it could be good to extend that semantic patch with another rule that would look for cases where a variable is never used, and then remove the declaration of the variable entirely. For this, the semantic patch rule shown in == Coccinelle challenge problem 2 can serve as inspiration, but needs to be somewhat modified. The goal is, as much as possible, to only remove a variable declaration when it becomes unnecessary due to the transformation described in challenge problem 1, not to remove all unused declarations like in challenge problem 2. |
In the following code, when x has any pointer type |
| Line 178: | Line 99: |
| Submit patches, including your semantic patch, based on your results. Does your semantic patch do too much? Think about what information you would need to get a better result. |
{{{ kfree((u8 *)x); }}} the cast to u8 *, or to any other pointer type is not needed. Write a semantic patch to remove such casts. Consider generalizing your semantic patch to functions other than kfree. Currently, the only occurrence of this problem for kfree is in the file wilc1000/host_interface.c. There is something else quite bizarre about this code. Consider how this other bizarre thing could be eliminated using Coccinelle. |
| Line 184: | Line 109: |
| The lustre file system in the staging tree defines the following macro: | Assignments in if conditions slightly complicate program analyses and are frowned upon by checkpatch. |
| Line 187: | Line 112: |
| #define GOTO(label, rc) \ do { \ if (cfs_cdebug_show(D_TRACE, DEBUG_SUBSYSTEM)) { \ LIBCFS_DEBUG_MSG_DATA_DECL(msgdata, D_TRACE, NULL); \ libcfs_log_goto(&msgdata, #label, (long_ptr_t)(rc)); \ } else { \ (void)(rc); \ } \ goto label; \ } while (0) |
if ((rc = pci_enable_device(pdev))) { printk(KERN_WARNING "i2o: couldn't enable device %s\n", pci_name(pdev)); return rc; } |
| Line 199: | Line 119: |
| In practice, the "then" branch of the if is debugging code, and it is only the code in the else branch that is useful, as well as the goto that is after the if. To put it another way, you can pretend that the test expression of the "if" always has the value false. The GOTO macro is not standard in Linux, and it would be nice to get rid of it. Write a semantic patch to make the required transformation. |
Write a semantic patch to move such assignments out before the if. In the general case, it may be necessary to take into account the possibility of operators such as && and ||. Your semantic patch should not change the order in which expressions are evaluated. In the case of very complex conditions, the transformation may also not be desirable, if it requires duplicating code or introducing many layers of ifs. |
| Line 208: | Line 123: |
| Continuing with the GOTO macro, you may find that in many cases the rc argument can be dropped completely. In what case should it be kept? In what case should it be dropped? Write a semantic patch that gives a pleasant result. <b>Hint:</b> it may be useful to consider the metavariable types identifier (a variable name like rc) and constant (a number like 0). <b>Bigger hint:</b> A number of patches making this transformation using Coccinelle have been submitted already. If you are stuck, track them down in the git logs and try to understand what they do. |
Assignments in function call arguments are also undesirable. Write a semantic patch to pull such assignments out before the function call. |
| Line 221: | Line 127: |
| The lustre file system in the staging tree uses a number of macros related to locks. Find their definitions and use Coccinelle to replace them by the corresponding standard Linux functions. == Coccinelle challenge problem 7 == 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. |
Some functions return NULL as a return value on failure. NULL can be tested for as !x, NULL == x, or x == NULL. When NULL represents failure, !x is commonly used. The following are some functions that commonly follow this strategy: |
| Line 230: | Line 130: |
| @@ expression x,e,e1; statement S; @@ if (x == 0) { ... when != x = e1 when != while(...) S when != for(...;...;...) S ( * x |= e | * x | e ) ... when any } |
kmalloc devm_kzalloc kmalloc_array devm_ioremap usb_alloc_urb alloc_netdev |
| Line 248: | Line 138: |
| 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. 1. 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 ...>. |
Write a semantic patch to clean up the tests on the results of one or more of these functions. |
| Line 251: | Line 140: |
| Note that there are very few occurrences of this problem in staging. If someone else has done this problem, you may need to look elsewhere in the kernel. == Coccinelle challenge problem 8 == The file include/linux/list.h contains some very useful functions for iterating over doubly linked lists. Some of these functions are ''list_for_each'', ''list_for_each_entry'', ''list_for_each_safe'', and ''list_for_each_entry_safe''. Some other functions related to doubly linked lists are ''list_empty'' and ''list_entry''. Sometimes when ''list_empty'' is used in a loop, the code could be better rewritten using eg ''list_for_each'', and sometimes when ''list_entry'' is used the code could be better written using eg ''list_for_each_entry''. Use Coccinelle to improve the code manipulating doubly linked lists, to remove when possible calls to ''list_empty'' and ''list_entry''. '''Note:''' This is a hard problem. You will need to study very carefully the definitions in include/linux/list.h and to study very carefully the code that uses these definitions, to be sure to be changing the code in the right way. Nevertheless, doubly linked lists are very widely used in the kernel, so it is useful to be familiar with how to manipulate them. == Coccinelle challenge problem 9 == Parentheses are not needed around the right hand side of an assignment, like in value = (FLASH_CMD_STATUS_REG_READ << 24);. Write a semantic patch to remove these parentheses. == Coccinelle challenge problem 10 == In the following code, when x has any pointer type kfree((u8 *)x); the cast to u8 *, or to any other pointer type is not needed. Write a semantic patch to remove such casts. Consider generalizing your semantic patch to functions other than kfree. |
As a much harder problem, use Coccinelle to find other functions for which tests for NULL use !x at least 70% of the time. |
| Line 277: | Line 144: |
| You can also try the [http://kernelnewbies.org/JuliaLawall_round8 Coccinelle challenge problems from round 8] and [http://kernelnewbies.org/JuliaLawall_round9 Coccinelle challenge problems from round 9]. | You can also try the [http://kernelnewbies.org/JuliaLawall_round8 Coccinelle challenge problems from round 8], [http://kernelnewbies.org/JuliaLawall_round9 Coccinelle challenge problems from round 9], and [http://kernelnewbies.org/JuliaLawall_round10 Coccinelle challenge problems from round 10]. |
About Me
I'm a researcher at Inria, in Paris France. I develop the tool [http://coccinelle.lip6.fr Coccinelle], which allows easy matching and transformation of C code. Coccinelle has been designed with the goal of contributing to Linux development, but it can also be used on other C code.
Please write to me directly if you would like to apply to the Coccinelle Outreachy project.
Overview
It would be a good idea to start with the first challenge problem, to check that you know how to use the tool properly. The remaining challenge problems can be done in any order. It is not obligatory to do all of them. You may find other things that can be done with Coccinelle. Sources of inspiration may be the results of checkpatch and patches that have been applied to the kernel in the past. Any kind of problem that occurs over and over might be amenable to being solved with Coccinelle.
These challenge problems may apply to many files in the kernel. Pick a few files, and send patches for those. Once they have been accepted, consider moving on to another challenge problem. You will get a better understanding of Coccinelle if you use it for many different things than if you use it do one thing over and over.
There are many examples of uses of Coccinelle, in previous patches, in the kernel source tree in the scripts/coccinelle directory, and at [https://github.com/coccinelle/coccinellery coccinellery]. If you use a script that is already in the Linux kernel, you don't need to include the script in your commit log, but rather something like Generate-by: sripts/coccinelle/misc/badty.cocci
Tutorial
A tutorial for Coccinelle is available [http://pagesperso-systeme.lip6.fr/Julia.Lawall/tutorial.pdf here]. These are slides that are intended to be presented, but they may be understandable independently of the presentation. Please note that the tutorial focuses on the source code of Linux 3.2, and so the patches created in doing the exercises of the tutorial are not suitable for submission to the opw-kernel mailing list. Doing the tutorial also does not count as a contribution to the project.
Coccinelle challenge problem 1
Consider the following function, from drivers/staging/most/hdm-dim2/dim2_sysfs.c
static ssize_t bus_kobj_attr_store(struct kobject *kobj, struct attribute *attr,
const char *buf, size_t count)
{
ssize_t ret;
struct medialb_bus *bus =
container_of(kobj, struct medialb_bus, kobj_group);
struct bus_attr *xattr = container_of(attr, struct bus_attr, attr);
if (!xattr->store)
return -EIO;
ret = xattr->store(bus, buf, count);
return ret;
}In this function, the last two lines could be compressed into one, as:
static ssize_t bus_kobj_attr_store(struct kobject *kobj, struct attribute *attr,
const char *buf, size_t count)
{
ssize_t ret;
struct medialb_bus *bus =
container_of(kobj, struct medialb_bus, kobj_group);
struct bus_attr *xattr = container_of(attr, struct bus_attr, attr);
if (!xattr->store)
return -EIO;
return xattr->store(bus, buf, count);
}The following semantic patch makes this change:
@@
expression e, ret;
@@
-ret =
+return
e;
-return ret;Do the following:
- Download and install Coccinelle. If you are using Linux, it should be available in your package manager. Any recent version is fine to start
with, but you may need to get the most recent version, which is 1.0.4. This is available on the Coccinelle webpage (coccinelle.lip6.fr) and on github.
- Download staging-testing
- Save the above semantic patch in a file ret.cocci
Run Coccinelle on ret.cocci and staging-testing, ie spatch --sp-file ret.cocci --no-includes --dir {your staging-testing path}/drivers/staging > ret.out. This may take some time.
Do you find the result satisfactory? If so, submit some patches. If not, let us know!
Your code may now declare some variables that are never used. Remove them before submitting your patch.
If you do submit a patch based on the use of Coccinelle, please mention Coccinelle in your patch, and the semantic patch that you used.
Coccinelle challenge problem 2
Parentheses are not needed around the right hand side of an assignment, like in value = (FLASH_CMD_STATUS_REG_READ << 24);. Write a semantic patch to remove these parentheses.
One could consider that parentheses might be useful in the case of eg rising = (dir == IIO_EV_DIR_RISING); because there could be a confusion between the different kinds of =. Extend your semantic patch using a disjunction so that it does not report on such cases.
Coccinelle challenge problem 3
In the following code, when x has any pointer type
kfree((u8 *)x);
the cast to u8 *, or to any other pointer type is not needed. Write a semantic patch to remove such casts. Consider generalizing your semantic patch to functions other than kfree.
Currently, the only occurrence of this problem for kfree is in the file wilc1000/host_interface.c. There is something else quite bizarre about this code. Consider how this other bizarre thing could be eliminated using Coccinelle.
Coccinelle challenge problem 4
Assignments in if conditions slightly complicate program analyses and are frowned upon by checkpatch.
if ((rc = pci_enable_device(pdev))) {
printk(KERN_WARNING "i2o: couldn't enable device %s\n",
pci_name(pdev));
return rc;
}Write a semantic patch to move such assignments out before the if. In the general case, it may be necessary to take into account the possibility of operators such as && and ||. Your semantic patch should not change the order in which expressions are evaluated. In the case of very complex conditions, the transformation may also not be desirable, if it requires duplicating code or introducing many layers of ifs.
Coccinelle challenge problem 5
Assignments in function call arguments are also undesirable. Write a semantic patch to pull such assignments out before the function call.
Coccinelle challenge problem 6
Some functions return NULL as a return value on failure. NULL can be tested for as !x, NULL == x, or x == NULL. When NULL represents failure, !x is commonly used. The following are some functions that commonly follow this strategy:
kmalloc devm_kzalloc kmalloc_array devm_ioremap usb_alloc_urb alloc_netdev
Write a semantic patch to clean up the tests on the results of one or more of these functions.
As a much harder problem, use Coccinelle to find other functions for which tests for NULL use !x at least 70% of the time.
Other Coccinelle challenge problems
You can also try the [http://kernelnewbies.org/JuliaLawall_round8 Coccinelle challenge problems from round 8], [http://kernelnewbies.org/JuliaLawall_round9 Coccinelle challenge problems from round 9], and [http://kernelnewbies.org/JuliaLawall_round10 Coccinelle challenge problems from round 10].
Contact info
Email: MailTo(Julia.Lawall AT lip6 DOT fr)
My IRC handle is jlawall.
Questions about using Coccinelle should go to the Coccinelle mailing list: MailTo(cocci AT systeme DOT lip6 DOT fr)