On Tue, 12 Mar 2019, Maxime Ripard wrote:
> Hi,
>
> I'm trying to do an API rework of DRM, and that rewrite involves
> patching a number of drivers to use a structure field instead of a
> funtion call.
>
> There's a bunch of cases that need to be covered, and I can't get all
> of them to work.
>
> So far, my current script is, with the current shortcomings as comment
> below each rule.
>
> @@
> identifier fn;
> identifier dev, cmd;
> @@
>
> fn (struct drm_device *dev,
> ...,
> struct drm_mode_fb_cmd2 *cmd,
> ...)
> {
> + const struct drm_format_info *info = drm_get_format_info(dev, cmd);
> ...
> - drm_format_num_planes(cmd->pixel_format)
> + info->num_planes
> ...
> }
>
> // This on works on most cases, ie:
> //
> https://elixir.bootlin.com/linux/v5.0/source/drivers/gpu/drm/armada/armada_fb.c#L87
> // However, for some reason unknown to me, it doesn't match:
> //
> https://elixir.bootlin.com/linux/v5.0/source/drivers/gpu/drm/tegra/fb.c#L129
... means by default that what comes before or after the ... should not
appear in the ... fb.c has the call in a loop, so it happens over and
over (... is following paths in the control-flow graph, not the AST.
Replace the first ... by <+... and the second one by ...+>. That will
replace all calls, ensuring that there is at least once to motivate the
need for adding the declaration of info.
>
> @@
> identifier fb;
> @@
> ...
> struct drm_framebuffer *fb;
> ...
> - drm_format_num_planes(fb->format->format)
> + fb->format->num_planes
>
> // This one seems to work properly
How about:
@@
struct drm_framebuffer *fb;
@@
- drm_format_num_planes(fb->format->format)
+ fb->format->num_planes
That is, you don't need to insist on there being a local variable
declaration, you just want an expression of the right type. This will
also be much more efficient.
> @@
> expression arg;
> identifier fb;
> @@
> ...
> struct drm_framebuffer *fb;
> ...
> - drm_format_num_planes(arg)
> + fb->format->num_planes
>
> // This one seems to work in some cases, such as
> //
> https://elixir.bootlin.com/linux/v5.0/source/drivers/gpu/drm/vc4/vc4_plane.c#L490
> // But it also matches in cases where fb hasn't been properly assigned
> before, such as:
> //
> https://elixir.bootlin.com/linux/v5.0/source/drivers/gpu/drm/tegra/fb.c#L142
OK, it looks like what you want is:
@@
struct drm_framebuffer *fb;
expression e.
@@
fb = e;
<...
- drm_format_num_planes(arg)
+ fb->format->num_planes
...>
That is, you find an assignment of fb, and then anywhere after that you
have a call, you can replace it. This is <... ...> rather than <+...
...+> so that it can match several 0 or more occurrences.
>
> @@
> identifier info;
> @@
> ...
> struct drm_format_info *info;
> ...
> - drm_format_num_planes(info->format)
> + info->num_planes
>
> // This one seems to work too
Still it would be better to update it as suggested above.
julia
> I'm pretty new to coccinelle, so I'm not quite sure how to fix these
> errors properly, even though that looks pretty simple.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
_______________________________________________
Cocci mailing list
[email protected]
https://systeme.lip6.fr/mailman/listinfo/cocci
