[Cocci] Inconsistent control-flow paths?

[Julia Lawall]

On Wed, 10 Mar 2010, Wolfram Sang wrote:

> Hello,
> 
> I'd need another explanation, please :) Still using this script:
> 
> ===
> 
> @@
> type T;
> identifier client, data;
> @@
> 
> // Check if function uses clientdata
> (
>         i2c_set_clientdata(client, data);
> |
>         data = i2c_get_clientdata(client);
> |
>         T data = i2c_get_clientdata(client);
> )
> // Anything inbetween
>         ...
> // Check if clientdata gets NULLed before data is freed
> (
>         i2c_set_clientdata(client, NULL);
>         ...
>         kfree(data);
> |
> +       i2c_set_clientdata(client, NULL);
>         kfree(data);
>         ...
> -       i2c_set_clientdata(client, NULL);
> |
> +       i2c_set_clientdata(client, NULL);
> ?       kfree(data);
> )
> 
> ===
> 
> In general, this works well and finds ~100 occurences, but I get a roughly a
> dozen inconsistent control-flow paths when applying this patch to the drivers/
> directory. Trying to figure out what is causing this, I just checked
> drivers/media/radio/radio-tea5764.c. I found '-show_trying' and it told me the
> problem is in tea5764_i2c_probe() which is what I expected, too. I also found
> '-allow_inconsistent_paths' which gave me the patch I wanted (but I don't want
> to use this function unless I know better what it does).

The problem is that the semantic patch is modifying the code that is in 
front of the kfree, at the label errfr.  This label is reachable from the 
top of your function, at the various occurrences of goto errfr, which 
occur before the call to i2c_set_clientdata that appears at the beginning 
of your semantic patch.  That label is also reachable from code that 
follows the call to i2c_set_clientdata.  Perhaps in this case you don't 
care, if setting data to NULL doesn't harm anything.  The more elegant 
thing though would be for the i2c_set_clientdata(client, NULL); not to be 
right before the kfree but to be right before the label errfr.  But 
Coccinelle is not clever enough to do that by itself, so it just points 
out the problem.

In this case, I don't really see a good solution.  You may as well fix it 
by hand.

julia