[PATCH 3/14] drivers/vhost/vhost.c: delete double assignment

[PATCH 3/14] drivers/vhost/vhost.c: delete double assignment

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Delete successive assignments to the same location.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression i;
@@

*i = ...;
 i = ...;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/vhost/vhost.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 94701ff..ed27727 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -157,7 +157,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
 	vq->avail_idx = 0;
 	vq->last_used_idx = 0;
 	vq->used_flags = 0;
-	vq->used_flags = 0;
 	vq->log_used = false;
 	vq->log_addr = -1ull;
 	vq->vhost_hlen = 0;

[PATCH 5/14] drivers/net/sb1000.c: delete double assignment

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

The other code around these duplicated assignments initializes the 0 1 2
and 3 elements of an array, so change the initialization of the
rx_session_id array to do the same.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression i;
@@

*i = ...;
 i = ...;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
This changes the semantics and has not been tested.

 drivers/net/sb1000.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
index a9ae505..66c2f1a 100644
--- a/drivers/net/sb1000.c
+++ b/drivers/net/sb1000.c
@@ -961,9 +961,9 @@ sb1000_open(struct net_device *dev)
 	lp->rx_error_count = 0;
 	lp->rx_error_dpc_count = 0;
 	lp->rx_session_id[0] = 0x50;
-	lp->rx_session_id[0] = 0x48;
-	lp->rx_session_id[0] = 0x44;
-	lp->rx_session_id[0] = 0x42;
+	lp->rx_session_id[1] = 0x48;
+	lp->rx_session_id[2] = 0x44;
+	lp->rx_session_id[3] = 0x42;
 	lp->rx_frame_id[0] = 0;
 	lp->rx_frame_id[1] = 0;
 	lp->rx_frame_id[2] = 0;

[PATCH 7/14] drivers/net/typhoon.c: delete double assignment

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Delete successive assignments to the same location.  The current definition
does not initialize the respRing structure, which has the same type as the
cmdRing structure, so initialize that one instead.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression i;
@@

*i = ...;
 i = ...;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
This changes the semantics and has not been tested.

 drivers/net/typhoon.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c
index 1cc6713..fc014eb 100644
--- a/drivers/net/typhoon.c
+++ b/drivers/net/typhoon.c
@@ -1328,7 +1328,7 @@ typhoon_init_rings(struct typhoon *tp)
 	tp->rxHiRing.lastWrite = 0;
 	tp->rxBuffRing.lastWrite = 0;
 	tp->cmdRing.lastWrite = 0;
-	tp->cmdRing.lastWrite = 0;
+	tp->respRing.lastWrite = 0;
 
 	tp->txLoRing.lastRead = 0;
 	tp->txHiRing.lastRead = 0;

[PATCH 10/14] drivers/isdn: delete double assignment

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Delete successive assignments to the same location.  In the first case, the
hscx array has two elements, so change the assignment to initialize the
second one.  In the second case, the two assignments are simply identical.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression i;
@@

*i = ...;
 i = ...;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
In the first case, the patch changes the semantics and has not been tested.

 drivers/isdn/hardware/mISDN/mISDNinfineon.c |    2 +-
 drivers/isdn/hisax/l3_1tr6.c                |    2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
index af25e1f..e90db88 100644
--- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
+++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
@@ -563,7 +563,7 @@ reset_inf(struct inf_hw *hw)
 		mdelay(10);
 		hw->ipac.isac.adf2 = 0x87;
 		hw->ipac.hscx[0].slot = 0x1f;
-		hw->ipac.hscx[0].slot = 0x23;
+		hw->ipac.hscx[1].slot = 0x23;
 		break;
 	case INF_GAZEL_R753:
 		val = inl((u32)hw->cfg.start + GAZEL_CNTRL);
diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
index b0554f8..a5c76fc 100644
--- a/drivers/isdn/hisax/l3_1tr6.c
+++ b/drivers/isdn/hisax/l3_1tr6.c
@@ -164,8 +164,6 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
 	char tmp[80];
 	struct sk_buff *skb = arg;
 
-	p = skb->data;
-
 	/* Channel Identification */
 	p = skb->data;
 	if ((p = findie(p, skb->len, WE0_chanID, 0))) {

Re: [PATCH 3/14] drivers/vhost/vhost.c: delete double assignment

[Michael S. Tsirkin]

On Tue, Oct 26, 2010 at 12:25:32PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> Delete successive assignments to the same location.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression i;
> @@
> 
> *i = ...;
>  i = ...;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Thanks, applied.

> ---
>  drivers/vhost/vhost.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 94701ff..ed27727 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -157,7 +157,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
>  	vq->avail_idx = 0;
>  	vq->last_used_idx = 0;
>  	vq->used_flags = 0;
> -	vq->used_flags = 0;
>  	vq->log_used = false;
>  	vq->log_addr = -1ull;
>  	vq->vhost_hlen = 0;

Re: [PATCH 7/14] drivers/net/typhoon.c: delete double assignment

[David Dillow]

On Tue, 2010-10-26 at 12:25 +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> Delete successive assignments to the same location.  The current definition
> does not initialize the respRing structure, which has the same type as the
> cmdRing structure, so initialize that one instead.

> Signed-off-by: Julia Lawall <julia@diku.dk>

Acked-by: David Dillow <dave@thedillows.org>

> ---
> This changes the semantics and has not been tested.

tp->respRing.lastWrite is only an artifact of using a common struct for
the rings and is not otherwise used, so no change to semantics.

Re: [PATCH 10/14] drivers/isdn: delete double assignment

[walter harms]

Julia Lawall schrieb:
> From: Julia Lawall <julia@diku.dk>
> 
> Delete successive assignments to the same location.  In the first case, the
> hscx array has two elements, so change the assignment to initialize the
> second one.  In the second case, the two assignments are simply identical.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression i;
> @@
> 
> *i = ...;
>  i = ...;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> In the first case, the patch changes the semantics and has not been tested.
> 
>  drivers/isdn/hardware/mISDN/mISDNinfineon.c |    2 +-
>  drivers/isdn/hisax/l3_1tr6.c                |    2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> index af25e1f..e90db88 100644
> --- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> +++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> @@ -563,7 +563,7 @@ reset_inf(struct inf_hw *hw)
>  		mdelay(10);
>  		hw->ipac.isac.adf2 = 0x87;
>  		hw->ipac.hscx[0].slot = 0x1f;
> -		hw->ipac.hscx[0].slot = 0x23;
> +		hw->ipac.hscx[1].slot = 0x23;
>  		break;
>  	case INF_GAZEL_R753:
>  		val = inl((u32)hw->cfg.start + GAZEL_CNTRL);
> diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
> index b0554f8..a5c76fc 100644
> --- a/drivers/isdn/hisax/l3_1tr6.c
> +++ b/drivers/isdn/hisax/l3_1tr6.c
> @@ -164,8 +164,6 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
>  	char tmp[80];
>  	struct sk_buff *skb = arg;
>  
> -	p = skb->data;
> -
>  	/* Channel Identification */
>  	p = skb->data;
>  	if ((p = findie(p, skb->len, WE0_chanID, 0))) {
> 


perhaps you can move the next assignment out of if also ?

p = findie(skb->data, skb->len, WE0_chanID, 0);
if (p) { ....


re,
 wh

> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 5/14] drivers/net/sb1000.c: delete double assignment

[walter harms]

Julia Lawall schrieb:
> From: Julia Lawall <julia@diku.dk>
> 
> The other code around these duplicated assignments initializes the 0 1 2
> and 3 elements of an array, so change the initialization of the
> rx_session_id array to do the same.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression i;
> @@
> 
> *i = ...;
>  i = ...;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> This changes the semantics and has not been tested.
> 
>  drivers/net/sb1000.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
> index a9ae505..66c2f1a 100644
> --- a/drivers/net/sb1000.c
> +++ b/drivers/net/sb1000.c
> @@ -961,9 +961,9 @@ sb1000_open(struct net_device *dev)
>  	lp->rx_error_count = 0;
>  	lp->rx_error_dpc_count = 0;
>  	lp->rx_session_id[0] = 0x50;
> -	lp->rx_session_id[0] = 0x48;
> -	lp->rx_session_id[0] = 0x44;
> -	lp->rx_session_id[0] = 0x42;
> +	lp->rx_session_id[1] = 0x48;
> +	lp->rx_session_id[2] = 0x44;
> +	lp->rx_session_id[3] = 0x42;
>  	lp->rx_frame_id[0] = 0;
>  	lp->rx_frame_id[1] = 0;
>  	lp->rx_frame_id[2] = 0;
> 

/me is surprised that this did not cause more problems.
Is it needed at all ?

re,
 wh

Re: [PATCH 10/14] drivers/isdn: delete double assignment

[Julia Lawall]

On Tue, 26 Oct 2010, walter harms wrote:

> 
> 
> Julia Lawall schrieb:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > Delete successive assignments to the same location.  In the first case, the
> > hscx array has two elements, so change the assignment to initialize the
> > second one.  In the second case, the two assignments are simply identical.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression i;
> > @@
> > 
> > *i = ...;
> >  i = ...;
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> > In the first case, the patch changes the semantics and has not been tested.
> > 
> >  drivers/isdn/hardware/mISDN/mISDNinfineon.c |    2 +-
> >  drivers/isdn/hisax/l3_1tr6.c                |    2 --
> >  2 files changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> > index af25e1f..e90db88 100644
> > --- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> > +++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> > @@ -563,7 +563,7 @@ reset_inf(struct inf_hw *hw)
> >  		mdelay(10);
> >  		hw->ipac.isac.adf2 = 0x87;
> >  		hw->ipac.hscx[0].slot = 0x1f;
> > -		hw->ipac.hscx[0].slot = 0x23;
> > +		hw->ipac.hscx[1].slot = 0x23;
> >  		break;
> >  	case INF_GAZEL_R753:
> >  		val = inl((u32)hw->cfg.start + GAZEL_CNTRL);
> > diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
> > index b0554f8..a5c76fc 100644
> > --- a/drivers/isdn/hisax/l3_1tr6.c
> > +++ b/drivers/isdn/hisax/l3_1tr6.c
> > @@ -164,8 +164,6 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
> >  	char tmp[80];
> >  	struct sk_buff *skb = arg;
> >  
> > -	p = skb->data;
> > -
> >  	/* Channel Identification */
> >  	p = skb->data;
> >  	if ((p = findie(p, skb->len, WE0_chanID, 0))) {
> > 
> 
> 
> perhaps you can move the next assignment out of if also ?
> 
> p = findie(skb->data, skb->len, WE0_chanID, 0);
> if (p) { ....

OK.

julia

Re: [PATCH 10/14] drivers/isdn: delete double assignment

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

Delete successive assignments to the same location.  In the first case, the
hscx array has two elements, so change the assignment to initialize the
second one.  In the second case, the two assignments are simply identical.
Furthermore, neither is necessary, because the effect of the assignment is
only visible in the next line, in the assignment in the if test.  The patch
inlines the right hand side value in the latter assignment and pulls that
assignment out of the if test.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression i;
@@

*i = ...;
 i = ...;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
In the first case, the patch changes the semantics and has not been tested.

 drivers/isdn/hardware/mISDN/mISDNinfineon.c |    2 +-
 drivers/isdn/hisax/l3_1tr6.c                |    6 ++----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
index af25e1f..e90db88 100644
--- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
+++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
@@ -563,7 +563,7 @@ reset_inf(struct inf_hw *hw)
 		mdelay(10);
 		hw->ipac.isac.adf2 = 0x87;
 		hw->ipac.hscx[0].slot = 0x1f;
-		hw->ipac.hscx[0].slot = 0x23;
+		hw->ipac.hscx[1].slot = 0x23;
 		break;
 	case INF_GAZEL_R753:
 		val = inl((u32)hw->cfg.start + GAZEL_CNTRL);
diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
index b0554f8..ee4dae1 100644
--- a/drivers/isdn/hisax/l3_1tr6.c
+++ b/drivers/isdn/hisax/l3_1tr6.c
@@ -164,11 +164,9 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
 	char tmp[80];
 	struct sk_buff *skb = arg;
 
-	p = skb->data;
-
 	/* Channel Identification */
-	p = skb->data;
-	if ((p = findie(p, skb->len, WE0_chanID, 0))) {
+	p = findie(skb->data, skb->len, WE0_chanID, 0);
+	if (p) {
 		if (p[1] != 1) {
 			l3_1tr6_error(pc, "setup wrong chanID len", skb);
 			return;

Re: [PATCH 5/14] drivers/net/sb1000.c: delete double assignment

[Julia Lawall]

On Tue, 26 Oct 2010, walter harms wrote:

> 
> 
> Julia Lawall schrieb:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > The other code around these duplicated assignments initializes the 0 1 2
> > and 3 elements of an array, so change the initialization of the
> > rx_session_id array to do the same.
> > 
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> > 
> > // <smpl>
> > @@
> > expression i;
> > @@
> > 
> > *i = ...;
> >  i = ...;
> > // </smpl>
> > 
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > ---
> > This changes the semantics and has not been tested.
> > 
> >  drivers/net/sb1000.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
> > index a9ae505..66c2f1a 100644
> > --- a/drivers/net/sb1000.c
> > +++ b/drivers/net/sb1000.c
> > @@ -961,9 +961,9 @@ sb1000_open(struct net_device *dev)
> >  	lp->rx_error_count = 0;
> >  	lp->rx_error_dpc_count = 0;
> >  	lp->rx_session_id[0] = 0x50;
> > -	lp->rx_session_id[0] = 0x48;
> > -	lp->rx_session_id[0] = 0x44;
> > -	lp->rx_session_id[0] = 0x42;
> > +	lp->rx_session_id[1] = 0x48;
> > +	lp->rx_session_id[2] = 0x44;
> > +	lp->rx_session_id[3] = 0x42;
> >  	lp->rx_frame_id[0] = 0;
> >  	lp->rx_frame_id[1] = 0;
> >  	lp->rx_frame_id[2] = 0;
> > 
> 
> /me is surprised that this did not cause more problems.
> Is it needed at all ?

The field appears to be useful.  However, there is an ioctl that also 
initializes the elements of this field with these values and that is 
implemented correctly.  Perhaps that was sufficient to hide the problem in 
practice.

julia

Re: [PATCH 5/14] drivers/net/sb1000.c: delete double assignment

[David Miller]

From: Julia Lawall <julia@diku.dk>
Date: Tue, 26 Oct 2010 12:25:34 +0200

> From: Julia Lawall <julia@diku.dk>
> 
> The other code around these duplicated assignments initializes the 0 1 2
> and 3 elements of an array, so change the initialization of the
> rx_session_id array to do the same.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <julia@diku.dk>

Applied.

Re: [PATCH 7/14] drivers/net/typhoon.c: delete double assignment

[David Miller]

From: Julia Lawall <julia@diku.dk>
Date: Tue, 26 Oct 2010 12:25:36 +0200

> From: Julia Lawall <julia@diku.dk>
> 
> Delete successive assignments to the same location.  The current definition
> does not initialize the respRing structure, which has the same type as the
> cmdRing structure, so initialize that one instead.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <julia@diku.dk>

Applied.

Re: [PATCH 10/14] drivers/isdn: delete double assignment

[David Miller]

From: Julia Lawall <julia@diku.dk>
Date: Tue, 26 Oct 2010 14:20:56 +0200 (CEST)

> From: Julia Lawall <julia@diku.dk>
> 
> Delete successive assignments to the same location.  In the first case, the
> hscx array has two elements, so change the assignment to initialize the
> second one.  In the second case, the two assignments are simply identical.
> Furthermore, neither is necessary, because the effect of the assignment is
> only visible in the next line, in the assignment in the if test.  The patch
> inlines the right hand side value in the latter assignment and pulls that
> assignment out of the if test.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <julia@diku.dk>

Applied.