<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 19, 2018 at 12:44 PM, Julia Lawall <span dir="ltr"><<a href="mailto:julia.lawall@lip6.fr" target="_blank">julia.lawall@lip6.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On Mon, 19 Mar 2018, Arushi Singhal wrote:<br>
<br>
> This patch replace list_entry with list_{next/prev}_entry as it makes<br>
> the code more clear to read.<br>
> Done using coccinelle:<br>
><br>
> @@<br>
> expression e1;<br>
> identifier e3;<br>
> type t;<br>
> @@<br>
> (<br>
> - list_entry(e1->e3.next,t,e3)<br>
> + list_next_entry(e1,e3)<br>
> |<br>
> - list_entry(e1->e3.prev,t,e3)<br>
> + list_prev_entry(e1,e3)<br>
> )<br>
<br>
</span>This looks like a rule that could be nice for the Linux kernel in general,<br>
because the code really is much simpler.<br>
<br>
I would suggest to write the rule in a more robust way, as follows:<br>
<br>
@@<br>
identifier e3;<br>
type t;<br>
t *e1;<br>
<span class="">@@<br>
<br>
(<br>
- list_entry(e1->e3.next,t,e3)<br>
+ list_next_entry(e1,e3)<br>
|<br>
- list_entry(e1->e3.prev,t,e3)<br>
+ list_prev_entry(e1,e3)<br>
)<br>
<br>
</span><span class="">@@<br>
expression e1;<br>
identifier e3;<br>
</span>@@<br>
<br>
(<br>
- list_entry(e1->e3.next,typeof(<wbr>*e1),e3)<br>
+ list_next_entry(e1,e3)<br>
|<br>
- list_entry(e1->e3.prev,typeof(<wbr>*e1),e3)<br>
+ list_prev_entry(e1,e3)<br>
<br>
This checks that the type that is specified corresponds to the one on e1.<br>
It could actually be that the call is getting the first element of a list,<br>
from some different type, and coincidentally the two types have the same<br>
field name for the list element.<br>
<br>
Unfortunately, the second rule, with the typeof call, doesn't currently<br>
work in Coccinelle, because the semantic patch language doesn't actually<br>
support typeof, and thinks that it is a function call. I will fix this.<br>
<br>
To make a semantic patch for the kernel, you can try running spgen on the<br>
above file and answer the questions that it asks. You can find examples<br>
in the coccinelle/scripts directory. Just run<br>
<br>
spgen foo.cocci<br>
<br>
Then answer the questions. Then run<br>
<br>
spgen foo.cocci > foo_for_kernel.cocci<br>
<br>
The second run will use the results of the first run to print the semantic<br>
patch. Let me know if you have any questions. You can always adjust the<br>
semantic patch that is generated by hand afterwards if needed.<br></blockquote><div><br></div><div>Hi Julia,<br><br></div><div>I tried spgen and found that second rule is still not working. It's not able to detect the second rule.<br></div><div>Is it working for you?<br><br></div><div>Thanks,<br></div><div>Arushi<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
julia<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> Signed-off-by: Arushi Singhal <<a href="mailto:arushisinghal19971997@gmail.com">arushisinghal19971997@gmail.<wbr>com</a>><br>
> ---<br>
> drivers/gpu/drm/drm_lease.c | 2 +-<br>
> drivers/gpu/drm/nouveau/nvkm/<wbr>subdev/clk/base.c | 2 +-<br>
> 2 files changed, 2 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c<br>
> index 1402c0e..4dcfb5f 100644<br>
> --- a/drivers/gpu/drm/drm_lease.c<br>
> +++ b/drivers/gpu/drm/drm_lease.c<br>
> @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct drm_master *top)<br>
> break;<br>
><br>
> /* Over */<br>
> - master = list_entry(master->lessee_<wbr>list.next, struct drm_master, lessee_list);<br>
> + master = list_next_entry(master, lessee_list);<br>
> }<br>
> }<br>
> }<br>
> diff --git a/drivers/gpu/drm/nouveau/<wbr>nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/<wbr>nvkm/subdev/clk/base.c<br>
> index e4c8d31..81c3567 100644<br>
> --- a/drivers/gpu/drm/nouveau/<wbr>nvkm/subdev/clk/base.c<br>
> +++ b/drivers/gpu/drm/nouveau/<wbr>nvkm/subdev/clk/base.c<br>
> @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,<br>
> nvkm_volt_map(volt, volt->max2_id, clk->temp));<br>
><br>
> for (cstate = start; &cstate->head != &pstate->list;<br>
> - cstate = list_entry(cstate->head.prev, typeof(*cstate), head)) {<br>
> + cstate = list_prev_entry(cstate, head)) {<br>
> if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))<br>
> break;<br>
> }<br>
> --<br>
> 2.7.4<br>
><br>
</div></div><span class="HOEnZb"><font color="#888888">> --<br>
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.<br>
> To unsubscribe from this group and stop receiving emails from it, send an email to <a href="mailto:outreachy-kernel%2Bunsubscribe@googlegroups.com">outreachy-kernel+unsubscribe@<wbr>googlegroups.com</a>.<br>
> To post to this group, send email to <a href="mailto:outreachy-kernel@googlegroups.com">outreachy-kernel@googlegroups.<wbr>com</a>.<br>
> To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/outreachy-kernel/20180319050530.GA25589%40seema-Inspiron-15-3567" rel="noreferrer" target="_blank">https://groups.google.com/d/<wbr>msgid/outreachy-kernel/<wbr>20180319050530.GA25589%<wbr>40seema-Inspiron-15-3567</a>.<br>
> For more options, visit <a href="https://groups.google.com/d/optout" rel="noreferrer" target="_blank">https://groups.google.com/d/<wbr>optout</a>.<br>
><br>
</font></span></blockquote></div><br></div></div>