[PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[Haneen Mohammed]

Replace use of the combination of list_for_each and list_entry
with list_for_each_entry to simplify the code and remove variables
that are used only in list_for_each.
Issue found and corrected using Coccinelle script:

@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@

-T1 *pos;
...when!=pos=e;

-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{
...when!=pos=e;
   when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos=e;
}
...when!=pos=e;

Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
---
 drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
index 3df101b..b8eba33 100644
--- a/drivers/staging/lustre/lnet/lnet/router.c
+++ b/drivers/staging/lustre/lnet/lnet/router.c
@@ -222,15 +222,12 @@ struct lnet_remotenet *
 lnet_find_net_locked(__u32 net)
 {
 	struct lnet_remotenet *rnet;
-	struct list_head *tmp;
 	struct list_head *rn_list;
 
 	LASSERT(!the_lnet.ln_shutdown);
 
 	rn_list = lnet_net2rnethash(net);
-	list_for_each(tmp, rn_list) {
-		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
-
+	list_for_each_entry(rnet, rn_list, lrn_list) {
 		if (rnet->lrn_net == net)
 			return rnet;
 	}
@@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
 	__u32 lnd_type, seed[2];
 	struct timespec64 ts;
 	struct lnet_ni *ni;
-	struct list_head *tmp;
 
 	if (seeded)
 		return;
@@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
 	 * Nodes with small feet have little entropy
 	 * the NID for this node gives the most entropy in the low bits
 	 */
-	list_for_each(tmp, &the_lnet.ln_nis) {
-		ni = list_entry(tmp, struct lnet_ni, ni_list);
+	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
 		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
 
 		if (lnd_type != LOLND)
-- 
2.7.4

[lustre-devel] [Outreachy kernel] [PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[Julia Lawall]

On Fri, 15 Sep 2017, Haneen Mohammed wrote:

> Replace use of the combination of list_for_each and list_entry
> with list_for_each_entry to simplify the code and remove variables
> that are used only in list_for_each.
> Issue found and corrected using Coccinelle script:
>
> @r@
> expression head, member, e;
> type T1, T2, T3;
> iterator name list_for_each, list_for_each_entry;
> identifier pos, var;
> @@
>
> -T1 *pos;
> ...when!=pos=e;
>
> -list_for_each(pos, head)
> +list_for_each_entry(var, head, member)
> {
> ...when!=pos=e;
>    when!=T3 *var;
> -var = list_entry(pos, T2, member);
> ...when!=pos=e;
> }
> ...when!=pos=e;

Actually, one could consider that there should be when != pos, not when !=
pos=e, because you need that there are no references at all to pos to be
able to delete it.  But it's true that if pos is not initialized then
there should be no other kinds of references either.  The other
possibility for initialization would be eg

f(...,&pos,...)

You could be suspicious of &pos in general.  But anyway there is no &pos
in this code.

julia

> Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

> ---
>  drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
> index 3df101b..b8eba33 100644
> --- a/drivers/staging/lustre/lnet/lnet/router.c
> +++ b/drivers/staging/lustre/lnet/lnet/router.c
> @@ -222,15 +222,12 @@ struct lnet_remotenet *
>  lnet_find_net_locked(__u32 net)
>  {
>  	struct lnet_remotenet *rnet;
> -	struct list_head *tmp;
>  	struct list_head *rn_list;
>
>  	LASSERT(!the_lnet.ln_shutdown);
>
>  	rn_list = lnet_net2rnethash(net);
> -	list_for_each(tmp, rn_list) {
> -		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
> -
> +	list_for_each_entry(rnet, rn_list, lrn_list) {
>  		if (rnet->lrn_net == net)
>  			return rnet;
>  	}
> @@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
>  	__u32 lnd_type, seed[2];
>  	struct timespec64 ts;
>  	struct lnet_ni *ni;
> -	struct list_head *tmp;
>
>  	if (seeded)
>  		return;
> @@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
>  	 * Nodes with small feet have little entropy
>  	 * the NID for this node gives the most entropy in the low bits
>  	 */
> -	list_for_each(tmp, &the_lnet.ln_nis) {
> -		ni = list_entry(tmp, struct lnet_ni, ni_list);
> +	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
>  		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
>
>  		if (lnd_type != LOLND)
> --
> 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 at googlegroups.com.
> To post to this group, send email to outreachy-kernel at googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170916004148.GA25693%40Haneen.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[Julia Lawall]

On Fri, 15 Sep 2017, Haneen Mohammed wrote:

> Replace use of the combination of list_for_each and list_entry
> with list_for_each_entry to simplify the code and remove variables
> that are used only in list_for_each.
> Issue found and corrected using Coccinelle script:
>
> @r@
> expression head, member, e;
> type T1, T2, T3;
> iterator name list_for_each, list_for_each_entry;
> identifier pos, var;
> @@
>
> -T1 *pos;
> ...when!=pos=e;
>
> -list_for_each(pos, head)
> +list_for_each_entry(var, head, member)
> {
> ...when!=pos=e;
>    when!=T3 *var;
> -var = list_entry(pos, T2, member);
> ...when!=pos=e;
> }
> ...when!=pos=e;

Actually, one could consider that there should be when != pos, not when !=
pos=e, because you need that there are no references at all to pos to be
able to delete it.  But it's true that if pos is not initialized then
there should be no other kinds of references either.  The other
possibility for initialization would be eg

f(...,&pos,...)

You could be suspicious of &pos in general.  But anyway there is no &pos
in this code.

julia

> Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

> ---
>  drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
> index 3df101b..b8eba33 100644
> --- a/drivers/staging/lustre/lnet/lnet/router.c
> +++ b/drivers/staging/lustre/lnet/lnet/router.c
> @@ -222,15 +222,12 @@ struct lnet_remotenet *
>  lnet_find_net_locked(__u32 net)
>  {
>  	struct lnet_remotenet *rnet;
> -	struct list_head *tmp;
>  	struct list_head *rn_list;
>
>  	LASSERT(!the_lnet.ln_shutdown);
>
>  	rn_list = lnet_net2rnethash(net);
> -	list_for_each(tmp, rn_list) {
> -		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
> -
> +	list_for_each_entry(rnet, rn_list, lrn_list) {
>  		if (rnet->lrn_net == net)
>  			return rnet;
>  	}
> @@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
>  	__u32 lnd_type, seed[2];
>  	struct timespec64 ts;
>  	struct lnet_ni *ni;
> -	struct list_head *tmp;
>
>  	if (seeded)
>  		return;
> @@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
>  	 * Nodes with small feet have little entropy
>  	 * the NID for this node gives the most entropy in the low bits
>  	 */
> -	list_for_each(tmp, &the_lnet.ln_nis) {
> -		ni = list_entry(tmp, struct lnet_ni, ni_list);
> +	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
>  		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
>
>  		if (lnd_type != LOLND)
> --
> 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/20170916004148.GA25693%40Haneen.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[Haneen Mohammed]

On Sat, Sep 16, 2017 at 01:23:28PM +0200, Julia Lawall wrote:
> 
> 
> On Fri, 15 Sep 2017, Haneen Mohammed wrote:
> 
> > Replace use of the combination of list_for_each and list_entry
> > with list_for_each_entry to simplify the code and remove variables
> > that are used only in list_for_each.
> > Issue found and corrected using Coccinelle script:
> >
> > @r@
> > expression head, member, e;
> > type T1, T2, T3;
> > iterator name list_for_each, list_for_each_entry;
> > identifier pos, var;
> > @@
> >
> > -T1 *pos;
> > ...when!=pos=e;
> >
> > -list_for_each(pos, head)
> > +list_for_each_entry(var, head, member)
> > {
> > ...when!=pos=e;
> >    when!=T3 *var;
> > -var = list_entry(pos, T2, member);
> > ...when!=pos=e;
> > }
> > ...when!=pos=e;
> 
> Actually, one could consider that there should be when != pos, not when !=
> pos=e, because you need that there are no references at all to pos to be
> able to delete it.  But it's true that if pos is not initialized then
> there should be no other kinds of references either.  The other
> possibility for initialization would be eg
> 
> f(...,&pos,...)
> 
> You could be suspicious of &pos in general.  But anyway there is no &pos
> in this code.
> 
> julia
> 

I have tried "when != pos;" first and switched to "when !=pos=e;"
becuase I have found cases where pos was still used. I only realised I
should have used "when != pos" without the semicolon after I've
submitted the patch.

There is still case where even with "when!=pos" the script fails to match pos
within an if statement. Is there away around that?

> > Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> 
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> 
> > ---
> >  drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
> > index 3df101b..b8eba33 100644
> > --- a/drivers/staging/lustre/lnet/lnet/router.c
> > +++ b/drivers/staging/lustre/lnet/lnet/router.c
> > @@ -222,15 +222,12 @@ struct lnet_remotenet *
> >  lnet_find_net_locked(__u32 net)
> >  {
> >  	struct lnet_remotenet *rnet;
> > -	struct list_head *tmp;
> >  	struct list_head *rn_list;
> >
> >  	LASSERT(!the_lnet.ln_shutdown);
> >
> >  	rn_list = lnet_net2rnethash(net);
> > -	list_for_each(tmp, rn_list) {
> > -		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
> > -
> > +	list_for_each_entry(rnet, rn_list, lrn_list) {
> >  		if (rnet->lrn_net == net)
> >  			return rnet;
> >  	}
> > @@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
> >  	__u32 lnd_type, seed[2];
> >  	struct timespec64 ts;
> >  	struct lnet_ni *ni;
> > -	struct list_head *tmp;
> >
> >  	if (seeded)
> >  		return;
> > @@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
> >  	 * Nodes with small feet have little entropy
> >  	 * the NID for this node gives the most entropy in the low bits
> >  	 */
> > -	list_for_each(tmp, &the_lnet.ln_nis) {
> > -		ni = list_entry(tmp, struct lnet_ni, ni_list);
> > +	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
> >  		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
> >
> >  		if (lnd_type != LOLND)
> > --
> > 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/20170916004148.GA25693%40Haneen.
> > For more options, visit https://groups.google.com/d/optout.
> >

Re: [Outreachy kernel] [PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[Julia Lawall]

On Sat, 16 Sep 2017, Haneen Mohammed wrote:

> On Sat, Sep 16, 2017 at 01:23:28PM +0200, Julia Lawall wrote:
> >
> >
> > On Fri, 15 Sep 2017, Haneen Mohammed wrote:
> >
> > > Replace use of the combination of list_for_each and list_entry
> > > with list_for_each_entry to simplify the code and remove variables
> > > that are used only in list_for_each.
> > > Issue found and corrected using Coccinelle script:
> > >
> > > @r@
> > > expression head, member, e;
> > > type T1, T2, T3;
> > > iterator name list_for_each, list_for_each_entry;
> > > identifier pos, var;
> > > @@
> > >
> > > -T1 *pos;
> > > ...when!=pos=e;
> > >
> > > -list_for_each(pos, head)
> > > +list_for_each_entry(var, head, member)
> > > {
> > > ...when!=pos=e;
> > >    when!=T3 *var;
> > > -var = list_entry(pos, T2, member);
> > > ...when!=pos=e;
> > > }
> > > ...when!=pos=e;
> >
> > Actually, one could consider that there should be when != pos, not when !=
> > pos=e, because you need that there are no references at all to pos to be
> > able to delete it.  But it's true that if pos is not initialized then
> > there should be no other kinds of references either.  The other
> > possibility for initialization would be eg
> >
> > f(...,&pos,...)
> >
> > You could be suspicious of &pos in general.  But anyway there is no &pos
> > in this code.
> >
> > julia
> >
>
> I have tried "when != pos;" first and switched to "when !=pos=e;"
> becuase I have found cases where pos was still used. I only realised I
> should have used "when != pos" without the semicolon after I've
> submitted the patch.
>
> There is still case where even with "when!=pos" the script fails to match pos
> within an if statement. Is there away around that?

Could you send an example?

julia


>
> > > Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> >
> > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> >
> > > ---
> > >  drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
> > >  1 file changed, 2 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
> > > index 3df101b..b8eba33 100644
> > > --- a/drivers/staging/lustre/lnet/lnet/router.c
> > > +++ b/drivers/staging/lustre/lnet/lnet/router.c
> > > @@ -222,15 +222,12 @@ struct lnet_remotenet *
> > >  lnet_find_net_locked(__u32 net)
> > >  {
> > >  	struct lnet_remotenet *rnet;
> > > -	struct list_head *tmp;
> > >  	struct list_head *rn_list;
> > >
> > >  	LASSERT(!the_lnet.ln_shutdown);
> > >
> > >  	rn_list = lnet_net2rnethash(net);
> > > -	list_for_each(tmp, rn_list) {
> > > -		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
> > > -
> > > +	list_for_each_entry(rnet, rn_list, lrn_list) {
> > >  		if (rnet->lrn_net == net)
> > >  			return rnet;
> > >  	}
> > > @@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
> > >  	__u32 lnd_type, seed[2];
> > >  	struct timespec64 ts;
> > >  	struct lnet_ni *ni;
> > > -	struct list_head *tmp;
> > >
> > >  	if (seeded)
> > >  		return;
> > > @@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
> > >  	 * Nodes with small feet have little entropy
> > >  	 * the NID for this node gives the most entropy in the low bits
> > >  	 */
> > > -	list_for_each(tmp, &the_lnet.ln_nis) {
> > > -		ni = list_entry(tmp, struct lnet_ni, ni_list);
> > > +	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
> > >  		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
> > >
> > >  		if (lnd_type != LOLND)
> > > --
> > > 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/20170916004148.GA25693%40Haneen.
> > > 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/20170916161048.GA6020%40Haneen.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[Haneen Mohammed]

On Sat, Sep 16, 2017 at 06:14:25PM +0200, Julia Lawall wrote:
> 
> 
> On Sat, 16 Sep 2017, Haneen Mohammed wrote:
> 
> > On Sat, Sep 16, 2017 at 01:23:28PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Fri, 15 Sep 2017, Haneen Mohammed wrote:
> > >
> > > > Replace use of the combination of list_for_each and list_entry
> > > > with list_for_each_entry to simplify the code and remove variables
> > > > that are used only in list_for_each.
> > > > Issue found and corrected using Coccinelle script:
> > > >
> > > > @r@
> > > > expression head, member, e;
> > > > type T1, T2, T3;
> > > > iterator name list_for_each, list_for_each_entry;
> > > > identifier pos, var;
> > > > @@
> > > >
> > > > -T1 *pos;
> > > > ...when!=pos=e;
> > > >
> > > > -list_for_each(pos, head)
> > > > +list_for_each_entry(var, head, member)
> > > > {
> > > > ...when!=pos=e;
> > > >    when!=T3 *var;
> > > > -var = list_entry(pos, T2, member);
> > > > ...when!=pos=e;
> > > > }
> > > > ...when!=pos=e;
> > >
> > > Actually, one could consider that there should be when != pos, not when !=
> > > pos=e, because you need that there are no references at all to pos to be
> > > able to delete it.  But it's true that if pos is not initialized then
> > > there should be no other kinds of references either.  The other
> > > possibility for initialization would be eg
> > >
> > > f(...,&pos,...)
> > >
> > > You could be suspicious of &pos in general.  But anyway there is no &pos
> > > in this code.
> > >
> > > julia
> > >
> >
> > I have tried "when != pos;" first and switched to "when !=pos=e;"
> > becuase I have found cases where pos was still used. I only realised I
> > should have used "when != pos" without the semicolon after I've
> > submitted the patch.
> >
> > There is still case where even with "when!=pos" the script fails to match pos
> > within an if statement. Is there away around that?
> 
> Could you send an example?
> 
> julia
> 
> 

in staging/lustre/lustre/ldlm/ldm_lock.c, line 898:

list_for_each(tmp, queue) {
	lock = list_entry(tmp, struct ldlm_lock, l_res_link);

	mode_end = list_prev_entry(lock, l_sl_mode);

	if (lock->l_req_mode != req->l_req_mode) {
		/* jump to last lock of mode group */
		tmp = &mode_end->l_res_link;
		continue;
	}
> >
> > > > Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> > >
> > > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> > >
> > > > ---
> > > >  drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
> > > >  1 file changed, 2 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
> > > > index 3df101b..b8eba33 100644
> > > > --- a/drivers/staging/lustre/lnet/lnet/router.c
> > > > +++ b/drivers/staging/lustre/lnet/lnet/router.c
> > > > @@ -222,15 +222,12 @@ struct lnet_remotenet *
> > > >  lnet_find_net_locked(__u32 net)
> > > >  {
> > > >  	struct lnet_remotenet *rnet;
> > > > -	struct list_head *tmp;
> > > >  	struct list_head *rn_list;
> > > >
> > > >  	LASSERT(!the_lnet.ln_shutdown);
> > > >
> > > >  	rn_list = lnet_net2rnethash(net);
> > > > -	list_for_each(tmp, rn_list) {
> > > > -		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
> > > > -
> > > > +	list_for_each_entry(rnet, rn_list, lrn_list) {
> > > >  		if (rnet->lrn_net == net)
> > > >  			return rnet;
> > > >  	}
> > > > @@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
> > > >  	__u32 lnd_type, seed[2];
> > > >  	struct timespec64 ts;
> > > >  	struct lnet_ni *ni;
> > > > -	struct list_head *tmp;
> > > >
> > > >  	if (seeded)
> > > >  		return;
> > > > @@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
> > > >  	 * Nodes with small feet have little entropy
> > > >  	 * the NID for this node gives the most entropy in the low bits
> > > >  	 */
> > > > -	list_for_each(tmp, &the_lnet.ln_nis) {
> > > > -		ni = list_entry(tmp, struct lnet_ni, ni_list);
> > > > +	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
> > > >  		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
> > > >
> > > >  		if (lnd_type != LOLND)
> > > > --
> > > > 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/20170916004148.GA25693%40Haneen.
> > > > 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/20170916161048.GA6020%40Haneen.
> > For more options, visit https://groups.google.com/d/optout.
> >

Re: [Outreachy kernel] [PATCH] staging: lustre: lnet: Replace list_for_each with list_for_each_entry

[Julia Lawall]

On Sat, 16 Sep 2017, Haneen Mohammed wrote:

> On Sat, Sep 16, 2017 at 06:14:25PM +0200, Julia Lawall wrote:
> >
> >
> > On Sat, 16 Sep 2017, Haneen Mohammed wrote:
> >
> > > On Sat, Sep 16, 2017 at 01:23:28PM +0200, Julia Lawall wrote:
> > > >
> > > >
> > > > On Fri, 15 Sep 2017, Haneen Mohammed wrote:
> > > >
> > > > > Replace use of the combination of list_for_each and list_entry
> > > > > with list_for_each_entry to simplify the code and remove variables
> > > > > that are used only in list_for_each.
> > > > > Issue found and corrected using Coccinelle script:
> > > > >
> > > > > @r@
> > > > > expression head, member, e;
> > > > > type T1, T2, T3;
> > > > > iterator name list_for_each, list_for_each_entry;
> > > > > identifier pos, var;
> > > > > @@
> > > > >
> > > > > -T1 *pos;
> > > > > ...when!=pos=e;
> > > > >
> > > > > -list_for_each(pos, head)
> > > > > +list_for_each_entry(var, head, member)
> > > > > {
> > > > > ...when!=pos=e;
> > > > >    when!=T3 *var;
> > > > > -var = list_entry(pos, T2, member);
> > > > > ...when!=pos=e;
> > > > > }
> > > > > ...when!=pos=e;
> > > >
> > > > Actually, one could consider that there should be when != pos, not when !=
> > > > pos=e, because you need that there are no references at all to pos to be
> > > > able to delete it.  But it's true that if pos is not initialized then
> > > > there should be no other kinds of references either.  The other
> > > > possibility for initialization would be eg
> > > >
> > > > f(...,&pos,...)
> > > >
> > > > You could be suspicious of &pos in general.  But anyway there is no &pos
> > > > in this code.
> > > >
> > > > julia
> > > >
> > >
> > > I have tried "when != pos;" first and switched to "when !=pos=e;"
> > > becuase I have found cases where pos was still used. I only realised I
> > > should have used "when != pos" without the semicolon after I've
> > > submitted the patch.
> > >
> > > There is still case where even with "when!=pos" the script fails to match pos
> > > within an if statement. Is there away around that?
> >
> > Could you send an example?
> >
> > julia
> >
> >
>
> in staging/lustre/lustre/ldlm/ldm_lock.c, line 898:
>
> list_for_each(tmp, queue) {
> 	lock = list_entry(tmp, struct ldlm_lock, l_res_link);
>
> 	mode_end = list_prev_entry(lock, l_sl_mode);
>
> 	if (lock->l_req_mode != req->l_req_mode) {
> 		/* jump to last lock of mode group */
> 		tmp = &mode_end->l_res_link;
> 		continue;
> 	}

Indeed, it should not match that.  Perhaps the problem is because of the
continue.  Thanks for the report.

julia


> > >
> > > > > Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > >
> > > > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> > > >
> > > > > ---
> > > > >  drivers/staging/lustre/lnet/lnet/router.c | 9 ++-------
> > > > >  1 file changed, 2 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/lustre/lnet/lnet/router.c b/drivers/staging/lustre/lnet/lnet/router.c
> > > > > index 3df101b..b8eba33 100644
> > > > > --- a/drivers/staging/lustre/lnet/lnet/router.c
> > > > > +++ b/drivers/staging/lustre/lnet/lnet/router.c
> > > > > @@ -222,15 +222,12 @@ struct lnet_remotenet *
> > > > >  lnet_find_net_locked(__u32 net)
> > > > >  {
> > > > >  	struct lnet_remotenet *rnet;
> > > > > -	struct list_head *tmp;
> > > > >  	struct list_head *rn_list;
> > > > >
> > > > >  	LASSERT(!the_lnet.ln_shutdown);
> > > > >
> > > > >  	rn_list = lnet_net2rnethash(net);
> > > > > -	list_for_each(tmp, rn_list) {
> > > > > -		rnet = list_entry(tmp, struct lnet_remotenet, lrn_list);
> > > > > -
> > > > > +	list_for_each_entry(rnet, rn_list, lrn_list) {
> > > > >  		if (rnet->lrn_net == net)
> > > > >  			return rnet;
> > > > >  	}
> > > > > @@ -243,7 +240,6 @@ static void lnet_shuffle_seed(void)
> > > > >  	__u32 lnd_type, seed[2];
> > > > >  	struct timespec64 ts;
> > > > >  	struct lnet_ni *ni;
> > > > > -	struct list_head *tmp;
> > > > >
> > > > >  	if (seeded)
> > > > >  		return;
> > > > > @@ -254,8 +250,7 @@ static void lnet_shuffle_seed(void)
> > > > >  	 * Nodes with small feet have little entropy
> > > > >  	 * the NID for this node gives the most entropy in the low bits
> > > > >  	 */
> > > > > -	list_for_each(tmp, &the_lnet.ln_nis) {
> > > > > -		ni = list_entry(tmp, struct lnet_ni, ni_list);
> > > > > +	list_for_each_entry(ni, &the_lnet.ln_nis, ni_list) {
> > > > >  		lnd_type = LNET_NETTYP(LNET_NIDNET(ni->ni_nid));
> > > > >
> > > > >  		if (lnd_type != LOLND)
> > > > > --
> > > > > 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/20170916004148.GA25693%40Haneen.
> > > > > 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/20170916161048.GA6020%40Haneen.
> > > 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/20170916162124.GA6808%40Haneen.
> For more options, visit https://groups.google.com/d/optout.
>

[PATCH] staging: lustre: lnet: replace list_for_each with list_for_each_entry

[Haneen Mohammed]

Replace use of the combination of list_for_each() and list_entry() with
list_for_each_entry() to simplify the code and remove variables that are
used only in list_for_each().
Issue found and corrected using Coccinelle script:

@r@
expression head, member, e;
type T1, T2, T3;
iterator name list_for_each, list_for_each_entry;
identifier pos, var;
@@

-T1 *pos;
...when!=pos

-list_for_each(pos, head)
+list_for_each_entry(var, head, member)
{
...when!=pos
   when!=T3 *var;
-var = list_entry(pos, T2, member);
...when!=pos
}
...when!=pos

Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
---
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
index fbbd8a5..d0ee58d 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
@@ -176,12 +176,9 @@ struct ksock_peer *
 ksocknal_find_peer_locked(struct lnet_ni *ni, struct lnet_process_id id)
 {
 	struct list_head *peer_list = ksocknal_nid2peerlist(id.nid);
-	struct list_head *tmp;
 	struct ksock_peer *peer;
 
-	list_for_each(tmp, peer_list) {
-		peer = list_entry(tmp, struct ksock_peer, ksnp_list);
-
+	list_for_each_entry(peer, peer_list, ksnp_list) {
 		LASSERT(!peer->ksnp_closing);
 
 		if (peer->ksnp_ni != ni)
@@ -453,7 +450,6 @@ int
 ksocknal_add_peer(struct lnet_ni *ni, struct lnet_process_id id, __u32 ipaddr,
 		  int port)
 {
-	struct list_head *tmp;
 	struct ksock_peer *peer;
 	struct ksock_peer *peer2;
 	struct ksock_route *route;
@@ -491,9 +487,7 @@ ksocknal_add_peer(struct lnet_ni *ni, struct lnet_process_id id, __u32 ipaddr,
 	}
 
 	route2 = NULL;
-	list_for_each(tmp, &peer->ksnp_routes) {
-		route2 = list_entry(tmp, struct ksock_route, ksnr_list);
-
+	list_for_each_entry(route2, &peer->ksnp_routes, ksnr_list) {
 		if (route2->ksnr_ipaddr == ipaddr)
 			break;
 
@@ -1854,12 +1848,10 @@ ksocknal_query(struct lnet_ni *ni, lnet_nid_t nid, unsigned long *when)
 
 	peer = ksocknal_find_peer_locked(ni, id);
 	if (peer) {
-		struct list_head *tmp;
 		struct ksock_conn *conn;
 		int bufnob;
 
-		list_for_each(tmp, &peer->ksnp_conns) {
-			conn = list_entry(tmp, struct ksock_conn, ksnc_list);
+		list_for_each_entry(conn, &peer->ksnp_conns, ksnc_list) {
 			bufnob = conn->ksnc_sock->sk->sk_wmem_queued;
 
 			if (bufnob < conn->ksnc_tx_bufnob) {
-- 
2.7.4