[Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Arushi Singhal]

This patch replace list_entry with list_{next/prev}_entry as it makes
the code more clear to read.
Done using coccinelle:

@@
expression e1;
identifier e3;
type t;
@@
(
- list_entry(e1->e3.next,t,e3)
+ list_next_entry(e1,e3)
|
- list_entry(e1->e3.prev,t,e3)
+ list_prev_entry(e1,e3)
)

Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
---
 drivers/gpu/drm/drm_lease.c                    | 2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 1402c0e..4dcfb5f 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct drm_master *top)
 				break;
 
 			/* Over */
-			master = list_entry(master->lessee_list.next, struct drm_master, lessee_list);
+			master = list_next_entry(master, lessee_list);
 		}
 	}
 }
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
index e4c8d31..81c3567 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
@@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
 			       nvkm_volt_map(volt, volt->max2_id, clk->temp));
 
 	for (cstate = start; &cstate->head != &pstate->list;
-	     cstate = list_entry(cstate->head.prev, typeof(*cstate), head)) {
+	     cstate = list_prev_entry(cstate, head)) {
 		if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
 			break;
 	}
-- 
2.7.4

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To post to this group, send email to outreachy-kernel@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20180319050530.GA25589%40seema-Inspiron-15-3567.
For more options, visit https://groups.google.com/d/optout.

Re: [Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Julia Lawall]

On Mon, 19 Mar 2018, Arushi Singhal wrote:

> This patch replace list_entry with list_{next/prev}_entry as it makes
> the code more clear to read.
> Done using coccinelle:
>
> @@
> expression e1;
> identifier e3;
> type t;
> @@
> (
> - list_entry(e1->e3.next,t,e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,t,e3)
> + list_prev_entry(e1,e3)
> )

This looks like a rule that could be nice for the Linux kernel in general,
because the code really is much simpler.

I would suggest to write the rule in a more robust way, as follows:

@@
identifier e3;
type t;
t *e1;
@@

(
- list_entry(e1->e3.next,t,e3)
+ list_next_entry(e1,e3)
|
- list_entry(e1->e3.prev,t,e3)
+ list_prev_entry(e1,e3)
)

@@
expression e1;
identifier e3;
@@

(
- list_entry(e1->e3.next,typeof(*e1),e3)
+ list_next_entry(e1,e3)
|
- list_entry(e1->e3.prev,typeof(*e1),e3)
+ list_prev_entry(e1,e3)

This checks that the type that is specified corresponds to the one on e1.
It could actually be that the call is getting the first element of a list,
from some different type, and coincidentally the two types have the same
field name for the list element.

Unfortunately, the second rule, with the typeof call, doesn't currently
work in Coccinelle, because the semantic patch language doesn't actually
support typeof, and thinks that it is a function call.  I will fix this.

To make a semantic patch for the kernel, you can try running spgen on the
above file and answer the questions that it asks.  You can find examples
in the coccinelle/scripts directory.  Just run

spgen foo.cocci

Then answer the questions.  Then run

spgen foo.cocci > foo_for_kernel.cocci

The second run will use the results of the first run to print the semantic
patch.  Let me know if you have any questions.  You can always adjust the
semantic patch that is generated by hand afterwards if needed.

julia


>
> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
>  drivers/gpu/drm/drm_lease.c                    | 2 +-
>  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index 1402c0e..4dcfb5f 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct drm_master *top)
>  				break;
>
>  			/* Over */
> -			master = list_entry(master->lessee_list.next, struct drm_master, lessee_list);
> +			master = list_next_entry(master, lessee_list);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> index e4c8d31..81c3567 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
>  			       nvkm_volt_map(volt, volt->max2_id, clk->temp));
>
>  	for (cstate = start; &cstate->head != &pstate->list;
> -	     cstate = list_entry(cstate->head.prev, typeof(*cstate), head)) {
> +	     cstate = list_prev_entry(cstate, head)) {
>  		if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
>  			break;
>  	}
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20180319050530.GA25589%40seema-Inspiron-15-3567.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To post to this group, send email to outreachy-kernel@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.20.1803190805260.2864%40hadrien.
For more options, visit https://groups.google.com/d/optout.

Re: [Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Daniel Vetter]

On Mon, Mar 19, 2018 at 10:35:30AM +0530, Arushi Singhal wrote:
> This patch replace list_entry with list_{next/prev}_entry as it makes
> the code more clear to read.
> Done using coccinelle:
> 
> @@
> expression e1;
> identifier e3;
> type t;
> @@
> (
> - list_entry(e1->e3.next,t,e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,t,e3)
> + list_prev_entry(e1,e3)
> )
> 
> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>

Thanks for your patch. Looks correct, but for merge technical reasons can
you please split it into 2 patches? One for drm_lease.c, with a drm/lease:
prefix, and one for the nouveau driver change, with a nouveau: prefix.
Both patches need to be submitted to slightly different sets of
maintainers too, pls consult scripts/get_maintainers.pl

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_lease.c                    | 2 +-
>  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index 1402c0e..4dcfb5f 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct drm_master *top)
>  				break;
>  
>  			/* Over */
> -			master = list_entry(master->lessee_list.next, struct drm_master, lessee_list);
> +			master = list_next_entry(master, lessee_list);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> index e4c8d31..81c3567 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *pstate,
>  			       nvkm_volt_map(volt, volt->max2_id, clk->temp));
>  
>  	for (cstate = start; &cstate->head != &pstate->list;
> -	     cstate = list_entry(cstate->head.prev, typeof(*cstate), head)) {
> +	     cstate = list_prev_entry(cstate, head)) {
>  		if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
>  			break;
>  	}
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20180319050530.GA25589%40seema-Inspiron-15-3567.
> For more options, visit https://groups.google.com/d/optout.

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To post to this group, send email to outreachy-kernel@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20180319141304.GP14155%40phenom.ffwll.local.
For more options, visit https://groups.google.com/d/optout.

Re: [Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Arushi Singhal]

[-- Attachment #1: Type: text/plain, Size: 4885 bytes --]

On Mon, Mar 19, 2018 at 12:44 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:

>
>
> On Mon, 19 Mar 2018, Arushi Singhal wrote:
>
> > This patch replace list_entry with list_{next/prev}_entry as it makes
> > the code more clear to read.
> > Done using coccinelle:
> >
> > @@
> > expression e1;
> > identifier e3;
> > type t;
> > @@
> > (
> > - list_entry(e1->e3.next,t,e3)
> > + list_next_entry(e1,e3)
> > |
> > - list_entry(e1->e3.prev,t,e3)
> > + list_prev_entry(e1,e3)
> > )
>
> This looks like a rule that could be nice for the Linux kernel in general,
> because the code really is much simpler.
>
> I would suggest to write the rule in a more robust way, as follows:
>
> @@
> identifier e3;
> type t;
> t *e1;
> @@
>
> (
> - list_entry(e1->e3.next,t,e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,t,e3)
> + list_prev_entry(e1,e3)
> )
>
> @@
> expression e1;
> identifier e3;
> @@
>
> (
> - list_entry(e1->e3.next,typeof(*e1),e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,typeof(*e1),e3)
> + list_prev_entry(e1,e3)
>
> This checks that the type that is specified corresponds to the one on e1.
> It could actually be that the call is getting the first element of a list,
> from some different type, and coincidentally the two types have the same
> field name for the list element.
>
> Unfortunately, the second rule, with the typeof call, doesn't currently
> work in Coccinelle, because the semantic patch language doesn't actually
> support typeof, and thinks that it is a function call.  I will fix this.
>
> To make a semantic patch for the kernel, you can try running spgen on the
> above file and answer the questions that it asks.  You can find examples
> in the coccinelle/scripts directory.  Just run
>
> spgen foo.cocci
>
> Then answer the questions.  Then run
>
> spgen foo.cocci > foo_for_kernel.cocci
>
> The second run will use the results of the first run to print the semantic
> patch.  Let me know if you have any questions.  You can always adjust the
> semantic patch that is generated by hand afterwards if needed.
>
>
Thanks Julia for explanation and it helped me to learn about Spgen tool.
I'll make the changes and resend.

Arushi

> julia
>
>
> >
> > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_lease.c                    | 2 +-
> >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> > index 1402c0e..4dcfb5f 100644
> > --- a/drivers/gpu/drm/drm_lease.c
> > +++ b/drivers/gpu/drm/drm_lease.c
> > @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct drm_master *top)
> >                               break;
> >
> >                       /* Over */
> > -                     master = list_entry(master->lessee_list.next,
> struct drm_master, lessee_list);
> > +                     master = list_next_entry(master, lessee_list);
> >               }
> >       }
> >  }
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > index e4c8d31..81c3567 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate,
> >                              nvkm_volt_map(volt, volt->max2_id,
> clk->temp));
> >
> >       for (cstate = start; &cstate->head != &pstate->list;
> > -          cstate = list_entry(cstate->head.prev, typeof(*cstate),
> head)) {
> > +          cstate = list_prev_entry(cstate, head)) {
> >               if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> >                       break;
> >       }
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/
> msgid/outreachy-kernel/20180319050530.GA25589%40seema-Inspiron-15-3567.
> > For more options, visit https://groups.google.com/d/optout.
> >
>

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To post to this group, send email to outreachy-kernel@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF8-33LS%3D5obrhxtBUB97WSCSLNTxCTFH_oNN6EqUZE52g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: text/html, Size: 7159 bytes --]

Re: [Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Arushi Singhal]

[-- Attachment #1: Type: text/plain, Size: 4924 bytes --]

On Mon, Mar 19, 2018 at 12:44 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:

>
>
> On Mon, 19 Mar 2018, Arushi Singhal wrote:
>
> > This patch replace list_entry with list_{next/prev}_entry as it makes
> > the code more clear to read.
> > Done using coccinelle:
> >
> > @@
> > expression e1;
> > identifier e3;
> > type t;
> > @@
> > (
> > - list_entry(e1->e3.next,t,e3)
> > + list_next_entry(e1,e3)
> > |
> > - list_entry(e1->e3.prev,t,e3)
> > + list_prev_entry(e1,e3)
> > )
>
> This looks like a rule that could be nice for the Linux kernel in general,
> because the code really is much simpler.
>
> I would suggest to write the rule in a more robust way, as follows:
>
> @@
> identifier e3;
> type t;
> t *e1;
> @@
>
> (
> - list_entry(e1->e3.next,t,e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,t,e3)
> + list_prev_entry(e1,e3)
> )
>
> @@
> expression e1;
> identifier e3;
> @@
>
> (
> - list_entry(e1->e3.next,typeof(*e1),e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,typeof(*e1),e3)
> + list_prev_entry(e1,e3)
>
> This checks that the type that is specified corresponds to the one on e1.
> It could actually be that the call is getting the first element of a list,
> from some different type, and coincidentally the two types have the same
> field name for the list element.
>
> Unfortunately, the second rule, with the typeof call, doesn't currently
> work in Coccinelle, because the semantic patch language doesn't actually
> support typeof, and thinks that it is a function call.  I will fix this.
>
> To make a semantic patch for the kernel, you can try running spgen on the
> above file and answer the questions that it asks.  You can find examples
> in the coccinelle/scripts directory.  Just run
>
> spgen foo.cocci
>
> Then answer the questions.  Then run
>
> spgen foo.cocci > foo_for_kernel.cocci
>
> The second run will use the results of the first run to print the semantic
> patch.  Let me know if you have any questions.  You can always adjust the
> semantic patch that is generated by hand afterwards if needed.
>

Hi Julia,

I tried spgen and found that second rule is still not working. It's not
able to detect the second rule.
Is it working for you?

Thanks,
Arushi


> julia
>
>
> >
> > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_lease.c                    | 2 +-
> >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> > index 1402c0e..4dcfb5f 100644
> > --- a/drivers/gpu/drm/drm_lease.c
> > +++ b/drivers/gpu/drm/drm_lease.c
> > @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct drm_master *top)
> >                               break;
> >
> >                       /* Over */
> > -                     master = list_entry(master->lessee_list.next,
> struct drm_master, lessee_list);
> > +                     master = list_next_entry(master, lessee_list);
> >               }
> >       }
> >  }
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > index e4c8d31..81c3567 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct
> nvkm_pstate *pstate,
> >                              nvkm_volt_map(volt, volt->max2_id,
> clk->temp));
> >
> >       for (cstate = start; &cstate->head != &pstate->list;
> > -          cstate = list_entry(cstate->head.prev, typeof(*cstate),
> head)) {
> > +          cstate = list_prev_entry(cstate, head)) {
> >               if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> >                       break;
> >       }
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/
> msgid/outreachy-kernel/20180319050530.GA25589%40seema-Inspiron-15-3567.
> > For more options, visit https://groups.google.com/d/optout.
> >
>

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To post to this group, send email to outreachy-kernel@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF9jHrOFL4LosK5GdbAgpmQRd9%3D08eNzvfzYxFaeXk972g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: Type: text/html, Size: 7244 bytes --]

Re: [Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 5953 bytes --]



On Sun, 25 Mar 2018, Arushi Singhal wrote:

>
>
> On Mon, Mar 19, 2018 at 12:44 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
>       On Mon, 19 Mar 2018, Arushi Singhal wrote:
>
>       > This patch replace list_entry with list_{next/prev}_entry as
>       it makes
>       > the code more clear to read.
>       > Done using coccinelle:
>       >
>       > @@
>       > expression e1;
>       > identifier e3;
>       > type t;
>       > @@
>       > (
>       > - list_entry(e1->e3.next,t,e3)
>       > + list_next_entry(e1,e3)
>       > |
>       > - list_entry(e1->e3.prev,t,e3)
>       > + list_prev_entry(e1,e3)
>       > )
>
>       This looks like a rule that could be nice for the Linux kernel
>       in general,
>       because the code really is much simpler.
>
>       I would suggest to write the rule in a more robust way, as
>       follows:
>
>       @@
>       identifier e3;
>       type t;
>       t *e1;
>       @@
>
>       (
>       - list_entry(e1->e3.next,t,e3)
>       + list_next_entry(e1,e3)
>       |
>       - list_entry(e1->e3.prev,t,e3)
>       + list_prev_entry(e1,e3)
>       )
>
>       @@
>       expression e1;
>       identifier e3;
>       @@
>
>       (
>       - list_entry(e1->e3.next,typeof(*e1),e3)
>       + list_next_entry(e1,e3)
>       |
>       - list_entry(e1->e3.prev,typeof(*e1),e3)
>       + list_prev_entry(e1,e3)
>
>       This checks that the type that is specified corresponds to the
>       one on e1.
>       It could actually be that the call is getting the first element
>       of a list,
>       from some different type, and coincidentally the two types have
>       the same
>       field name for the list element.
>
>       Unfortunately, the second rule, with the typeof call, doesn't
>       currently
>       work in Coccinelle, because the semantic patch language doesn't
>       actually
>       support typeof, and thinks that it is a function call.  I will
>       fix this.
>
>       To make a semantic patch for the kernel, you can try running
>       spgen on the
>       above file and answer the questions that it asks.  You can find
>       examples
>       in the coccinelle/scripts directory.  Just run
>
>       spgen foo.cocci
>
>       Then answer the questions.  Then run
>
>       spgen foo.cocci > foo_for_kernel.cocci
>
>       The second run will use the results of the first run to print
>       the semantic
>       patch.  Let me know if you have any questions.  You can always
>       adjust the
>       semantic patch that is generated by hand afterwards if needed.
>
>
> Hi Julia,
>
> I tried spgen and found that second rule is still not working. It's not able
> to detect the second rule.
> Is it working for you?

No, I didn't have a chance to fix it yet.

julia


>
> Thanks,
> Arushi
>
>
>       julia
>
>
>       >
>       > Signed-off-by: Arushi Singhal
>       <arushisinghal19971997@gmail.com>
>       > ---
>       >  drivers/gpu/drm/drm_lease.c                    | 2 +-
>       >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
>       >  2 files changed, 2 insertions(+), 2 deletions(-)
>       >
>       > diff --git a/drivers/gpu/drm/drm_lease.c
>       b/drivers/gpu/drm/drm_lease.c
>       > index 1402c0e..4dcfb5f 100644
>       > --- a/drivers/gpu/drm/drm_lease.c
>       > +++ b/drivers/gpu/drm/drm_lease.c
>       > @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct
>       drm_master *top)
>       >                               break;
>       >
>       >                       /* Over */
>       > -                     master =
>       list_entry(master->lessee_list.next, struct drm_master,
>       lessee_list);
>       > +                     master = list_next_entry(master,
>       lessee_list);
>       >               }
>       >       }
>       >  }
>       > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       > index e4c8d31..81c3567 100644
>       > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       > @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk
>       *clk, struct nvkm_pstate *pstate,
>       >                              nvkm_volt_map(volt,
>       volt->max2_id, clk->temp));
>       >
>       >       for (cstate = start; &cstate->head != &pstate->list;
>       > -          cstate = list_entry(cstate->head.prev,
>       typeof(*cstate), head)) {
>       > +          cstate = list_prev_entry(cstate, head)) {
>       >               if (nvkm_cstate_valid(clk, cstate, max_volt,
>       clk->temp))
>       >                       break;
>       >       }
>       > --
>       > 2.7.4
>       >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to
> outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20180319050530.GA25589%4
> 0seema-Inspiron-15-3567.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF9jHrOFL4LosK5Gd
> bAgpmQRd9%3D08eNzvfzYxFaeXk972g%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

Re: [Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 6134 bytes --]



On Sun, 25 Mar 2018, Arushi Singhal wrote:

>
>
> On Mon, Mar 19, 2018 at 12:44 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
>       On Mon, 19 Mar 2018, Arushi Singhal wrote:
>
>       > This patch replace list_entry with list_{next/prev}_entry as
>       it makes
>       > the code more clear to read.
>       > Done using coccinelle:
>       >
>       > @@
>       > expression e1;
>       > identifier e3;
>       > type t;
>       > @@
>       > (
>       > - list_entry(e1->e3.next,t,e3)
>       > + list_next_entry(e1,e3)
>       > |
>       > - list_entry(e1->e3.prev,t,e3)
>       > + list_prev_entry(e1,e3)
>       > )
>
>       This looks like a rule that could be nice for the Linux kernel
>       in general,
>       because the code really is much simpler.
>
>       I would suggest to write the rule in a more robust way, as
>       follows:
>
>       @@
>       identifier e3;
>       type t;
>       t *e1;
>       @@
>
>       (
>       - list_entry(e1->e3.next,t,e3)
>       + list_next_entry(e1,e3)
>       |
>       - list_entry(e1->e3.prev,t,e3)
>       + list_prev_entry(e1,e3)
>       )
>
>       @@
>       expression e1;
>       identifier e3;
>       @@
>
>       (
>       - list_entry(e1->e3.next,typeof(*e1),e3)
>       + list_next_entry(e1,e3)
>       |
>       - list_entry(e1->e3.prev,typeof(*e1),e3)
>       + list_prev_entry(e1,e3)
>
>       This checks that the type that is specified corresponds to the
>       one on e1.
>       It could actually be that the call is getting the first element
>       of a list,
>       from some different type, and coincidentally the two types have
>       the same
>       field name for the list element.
>
>       Unfortunately, the second rule, with the typeof call, doesn't
>       currently
>       work in Coccinelle, because the semantic patch language doesn't
>       actually
>       support typeof, and thinks that it is a function call.  I will
>       fix this.
>
>       To make a semantic patch for the kernel, you can try running
>       spgen on the
>       above file and answer the questions that it asks.  You can find
>       examples
>       in the coccinelle/scripts directory.  Just run
>
>       spgen foo.cocci
>
>       Then answer the questions.  Then run
>
>       spgen foo.cocci > foo_for_kernel.cocci
>
>       The second run will use the results of the first run to print
>       the semantic
>       patch.  Let me know if you have any questions.  You can always
>       adjust the
>       semantic patch that is generated by hand afterwards if needed.
>
>
> Hi Julia,
>
> I tried spgen and found that second rule is still not working. It's not able
> to detect the second rule.
> Is it working for you?

If you get the latest version of Coccinelle from github, it shoudl be
working now.

julia

>
> Thanks,
> Arushi
>
>
>       julia
>
>
>       >
>       > Signed-off-by: Arushi Singhal
>       <arushisinghal19971997@gmail.com>
>       > ---
>       >  drivers/gpu/drm/drm_lease.c                    | 2 +-
>       >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
>       >  2 files changed, 2 insertions(+), 2 deletions(-)
>       >
>       > diff --git a/drivers/gpu/drm/drm_lease.c
>       b/drivers/gpu/drm/drm_lease.c
>       > index 1402c0e..4dcfb5f 100644
>       > --- a/drivers/gpu/drm/drm_lease.c
>       > +++ b/drivers/gpu/drm/drm_lease.c
>       > @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct
>       drm_master *top)
>       >                               break;
>       >
>       >                       /* Over */
>       > -                     master =
>       list_entry(master->lessee_list.next, struct drm_master,
>       lessee_list);
>       > +                     master = list_next_entry(master,
>       lessee_list);
>       >               }
>       >       }
>       >  }
>       > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       > index e4c8d31..81c3567 100644
>       > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
>       > @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk
>       *clk, struct nvkm_pstate *pstate,
>       >                              nvkm_volt_map(volt,
>       volt->max2_id, clk->temp));
>       >
>       >       for (cstate = start; &cstate->head != &pstate->list;
>       > -          cstate = list_entry(cstate->head.prev,
>       typeof(*cstate), head)) {
>       > +          cstate = list_prev_entry(cstate, head)) {
>       >               if (nvkm_cstate_valid(clk, cstate, max_volt,
>       clk->temp))
>       >                       break;
>       >       }
>       > --
>       > 2.7.4
>       >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to
> outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20180319050530.GA25589%4
> 0seema-Inspiron-15-3567.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
>
>
>

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To post to this group, send email to outreachy-kernel@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.20.1803260044440.2488%40hadrien.
For more options, visit https://groups.google.com/d/optout.