Re: [cocci] behavior change in semantic patches since c36b51ffc889 …

[Jacob Keller <jacob.e.keller@intel.com>]

On 3/8/2025 1:07 AM, Markus Elfring wrote:
>> For what its worth, I was finally able to get things working pretty much
>> the same as before with the following semantic patches:
> 
> This is a nice feedback.
> 
> 
>>> @@
>>> expression hw;
>>> @@
>>> -ice_free
>>> +devm_kfree
>>>  (
>>> +ice_hw_to_dev(
>>>  hw
>>> +)
>>>  , ...)
> 
> Did you observe that the omission of a semicolon at the end
> triggered desirable effects?
> 

I'm not sure if that impacted things, but it may have.

> 
> 
>> and
>>
>>> @disable drop_cast@
>>> void * void_ptr;
>>> type T;
>>> @@
>>> -(T *)void_ptr
>>> +void_ptr
> 
> Would another SmPL code variant be occasionally helpful?
> 
> -(T *)
>  void_ptr
> 

I tried this version. The big issue here is that spatch will remove the
typecast, but tries to leave any code outside of -/+ alone. This results
in the types remaining on their old line when code is formatted like this:

var = (cast)
   value;

This is sometimes done to avoid line length issues. With the style I
used, we get:

var = value;

But with the style you propose here we get:

var =
   value;

Which is not always desirable.

I found that both styles can be helpful depending on what you're doing.

> 
> Are there cases to consider where casts should be preserved?
> 

Yes, I ran into this exact issue with this aggressive version because
there are some casts which do something like cast a void pointer with
immediate access, without saving it to a variable. This more aggressive
version ended up triggering on these.

We managed to get one that works with a little more context:

@disable drop_cast@
void * void_ptr;
type T;
T * typed_ptr;
@@
 typed_ptr =
-(T *)void_ptr
+void_ptr


This version puts the typed_ptr as just context. This change alone seems
to be sufficient to get coccinelle to work with both versions, while
still preserving the line transformations we desired as I mentioned above.

There were exactly 2 places in the code base where this produced
different results, with the form:

type var =
   (cast)value;

Because the "type var" is in a context line, it doesn't get changed and
squashed with the value.

We just modified these two places to put the cast on the first line so
it would get squashed appropriately.

> Regards,
> Markus