Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Joe Perches
On Thu, 2014-06-26 at 14:12 +0200, Maurizio Lombardi wrote:
> 
> On 06/26/2014 02:05 PM, Joe Perches wrote:
> > On Thu, 2014-06-26 at 13:54 +0200, Rickard Strandqvist wrote:
> >> A struct member variable is set to different values without having used in 
> >> between.
> > []
> >> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
> >> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> > []
> >> @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
> >> iscsi_cls_conn *cls_conn,
> >>stats->r2t_pdus = conn->r2t_pdus_cnt;
> >>stats->tmfcmd_pdus = conn->tmfcmd_pdus_cnt;
> >>stats->tmfrsp_pdus = conn->tmfrsp_pdus_cnt;
> >> -  stats->custom_length = 3;
> >>strcpy(stats->custom[2].desc, "eh_abort_cnt");
> >>stats->custom[2].value = conn->eh_abort_cnt;
> >>stats->digest_err = 0;
> >>stats->timeout_err = 0;
> >> -  stats->custom_length = 0;
> >> +  stats->custom_length = 3;
> > 
> > You are changing custom_length from 0 to 3.
> > 
> > Why is this correct?
> 
> http://marc.info/?l=linux-scsi=140371670511706=2

Then the subject and commit message are wrong and need
to be updated to reflect your text.

This is a bugfix, not a cleanup.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Maurizio Lombardi


On 06/26/2014 01:54 PM, Rickard Strandqvist wrote:
> A struct member variable is set to different values without having used in 
> between.

It is almost ok for me but I think you should mention that it also fixes a bug,
or the commit message will be misleading.

> 
> This was found using a static code analysis program called cppcheck
> 
> Signed-off-by: Rickard Strandqvist 
> ---
>  drivers/scsi/bnx2i/bnx2i_iscsi.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> index 166543f..4e17a7f 100644
> --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
> +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
> iscsi_cls_conn *cls_conn,
>   stats->r2t_pdus = conn->r2t_pdus_cnt;
>   stats->tmfcmd_pdus = conn->tmfcmd_pdus_cnt;
>   stats->tmfrsp_pdus = conn->tmfrsp_pdus_cnt;
> - stats->custom_length = 3;
>   strcpy(stats->custom[2].desc, "eh_abort_cnt");
>   stats->custom[2].value = conn->eh_abort_cnt;
>   stats->digest_err = 0;
>   stats->timeout_err = 0;
> - stats->custom_length = 0;
> + stats->custom_length = 3;
>  }
>  
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Maurizio Lombardi


On 06/26/2014 02:05 PM, Joe Perches wrote:
> On Thu, 2014-06-26 at 13:54 +0200, Rickard Strandqvist wrote:
>> A struct member variable is set to different values without having used in 
>> between.
> []
>> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
>> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> []
>> @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
>> iscsi_cls_conn *cls_conn,
>>  stats->r2t_pdus = conn->r2t_pdus_cnt;
>>  stats->tmfcmd_pdus = conn->tmfcmd_pdus_cnt;
>>  stats->tmfrsp_pdus = conn->tmfrsp_pdus_cnt;
>> -stats->custom_length = 3;
>>  strcpy(stats->custom[2].desc, "eh_abort_cnt");
>>  stats->custom[2].value = conn->eh_abort_cnt;
>>  stats->digest_err = 0;
>>  stats->timeout_err = 0;
>> -stats->custom_length = 0;
>> +stats->custom_length = 3;
> 
> You are changing custom_length from 0 to 3.
> 
> Why is this correct?

http://marc.info/?l=linux-scsi=140371670511706=2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Joe Perches
On Thu, 2014-06-26 at 13:54 +0200, Rickard Strandqvist wrote:
> A struct member variable is set to different values without having used in 
> between.
[]
> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
[]
> @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
> iscsi_cls_conn *cls_conn,
>   stats->r2t_pdus = conn->r2t_pdus_cnt;
>   stats->tmfcmd_pdus = conn->tmfcmd_pdus_cnt;
>   stats->tmfrsp_pdus = conn->tmfrsp_pdus_cnt;
> - stats->custom_length = 3;
>   strcpy(stats->custom[2].desc, "eh_abort_cnt");
>   stats->custom[2].value = conn->eh_abort_cnt;
>   stats->digest_err = 0;
>   stats->timeout_err = 0;
> - stats->custom_length = 0;
> + stats->custom_length = 3;

You are changing custom_length from 0 to 3.

Why is this correct?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Rickard Strandqvist
A struct member variable is set to different values without having used in 
between.

This was found using a static code analysis program called cppcheck

Signed-off-by: Rickard Strandqvist 
---
 drivers/scsi/bnx2i/bnx2i_iscsi.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index 166543f..4e17a7f 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct iscsi_cls_conn 
*cls_conn,
stats->r2t_pdus = conn->r2t_pdus_cnt;
stats->tmfcmd_pdus = conn->tmfcmd_pdus_cnt;
stats->tmfrsp_pdus = conn->tmfrsp_pdus_cnt;
-   stats->custom_length = 3;
strcpy(stats->custom[2].desc, "eh_abort_cnt");
stats->custom[2].value = conn->eh_abort_cnt;
stats->digest_err = 0;
stats->timeout_err = 0;
-   stats->custom_length = 0;
+   stats->custom_length = 3;
 }
 
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Rickard Strandqvist
A struct member variable is set to different values without having used in 
between.

This was found using a static code analysis program called cppcheck

Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
---
 drivers/scsi/bnx2i/bnx2i_iscsi.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index 166543f..4e17a7f 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct iscsi_cls_conn 
*cls_conn,
stats-r2t_pdus = conn-r2t_pdus_cnt;
stats-tmfcmd_pdus = conn-tmfcmd_pdus_cnt;
stats-tmfrsp_pdus = conn-tmfrsp_pdus_cnt;
-   stats-custom_length = 3;
strcpy(stats-custom[2].desc, eh_abort_cnt);
stats-custom[2].value = conn-eh_abort_cnt;
stats-digest_err = 0;
stats-timeout_err = 0;
-   stats-custom_length = 0;
+   stats-custom_length = 3;
 }
 
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Joe Perches
On Thu, 2014-06-26 at 13:54 +0200, Rickard Strandqvist wrote:
 A struct member variable is set to different values without having used in 
 between.
[]
 diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
 b/drivers/scsi/bnx2i/bnx2i_iscsi.c
[]
 @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
 iscsi_cls_conn *cls_conn,
   stats-r2t_pdus = conn-r2t_pdus_cnt;
   stats-tmfcmd_pdus = conn-tmfcmd_pdus_cnt;
   stats-tmfrsp_pdus = conn-tmfrsp_pdus_cnt;
 - stats-custom_length = 3;
   strcpy(stats-custom[2].desc, eh_abort_cnt);
   stats-custom[2].value = conn-eh_abort_cnt;
   stats-digest_err = 0;
   stats-timeout_err = 0;
 - stats-custom_length = 0;
 + stats-custom_length = 3;

You are changing custom_length from 0 to 3.

Why is this correct?


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Maurizio Lombardi


On 06/26/2014 02:05 PM, Joe Perches wrote:
 On Thu, 2014-06-26 at 13:54 +0200, Rickard Strandqvist wrote:
 A struct member variable is set to different values without having used in 
 between.
 []
 diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
 b/drivers/scsi/bnx2i/bnx2i_iscsi.c
 []
 @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
 iscsi_cls_conn *cls_conn,
  stats-r2t_pdus = conn-r2t_pdus_cnt;
  stats-tmfcmd_pdus = conn-tmfcmd_pdus_cnt;
  stats-tmfrsp_pdus = conn-tmfrsp_pdus_cnt;
 -stats-custom_length = 3;
  strcpy(stats-custom[2].desc, eh_abort_cnt);
  stats-custom[2].value = conn-eh_abort_cnt;
  stats-digest_err = 0;
  stats-timeout_err = 0;
 -stats-custom_length = 0;
 +stats-custom_length = 3;
 
 You are changing custom_length from 0 to 3.
 
 Why is this correct?

http://marc.info/?l=linux-scsim=140371670511706w=2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Maurizio Lombardi


On 06/26/2014 01:54 PM, Rickard Strandqvist wrote:
 A struct member variable is set to different values without having used in 
 between.

It is almost ok for me but I think you should mention that it also fixes a bug,
or the commit message will be misleading.

 
 This was found using a static code analysis program called cppcheck
 
 Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se
 ---
  drivers/scsi/bnx2i/bnx2i_iscsi.c |3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
 
 diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
 b/drivers/scsi/bnx2i/bnx2i_iscsi.c
 index 166543f..4e17a7f 100644
 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
 +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
 @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
 iscsi_cls_conn *cls_conn,
   stats-r2t_pdus = conn-r2t_pdus_cnt;
   stats-tmfcmd_pdus = conn-tmfcmd_pdus_cnt;
   stats-tmfrsp_pdus = conn-tmfrsp_pdus_cnt;
 - stats-custom_length = 3;
   strcpy(stats-custom[2].desc, eh_abort_cnt);
   stats-custom[2].value = conn-eh_abort_cnt;
   stats-digest_err = 0;
   stats-timeout_err = 0;
 - stats-custom_length = 0;
 + stats-custom_length = 3;
  }
  
  
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once

2014-06-26 Thread Joe Perches
On Thu, 2014-06-26 at 14:12 +0200, Maurizio Lombardi wrote:
 
 On 06/26/2014 02:05 PM, Joe Perches wrote:
  On Thu, 2014-06-26 at 13:54 +0200, Rickard Strandqvist wrote:
  A struct member variable is set to different values without having used in 
  between.
  []
  diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c 
  b/drivers/scsi/bnx2i/bnx2i_iscsi.c
  []
  @@ -1643,12 +1643,11 @@ static void bnx2i_conn_get_stats(struct 
  iscsi_cls_conn *cls_conn,
 stats-r2t_pdus = conn-r2t_pdus_cnt;
 stats-tmfcmd_pdus = conn-tmfcmd_pdus_cnt;
 stats-tmfrsp_pdus = conn-tmfrsp_pdus_cnt;
  -  stats-custom_length = 3;
 strcpy(stats-custom[2].desc, eh_abort_cnt);
 stats-custom[2].value = conn-eh_abort_cnt;
 stats-digest_err = 0;
 stats-timeout_err = 0;
  -  stats-custom_length = 0;
  +  stats-custom_length = 3;
  
  You are changing custom_length from 0 to 3.
  
  Why is this correct?
 
 http://marc.info/?l=linux-scsim=140371670511706w=2

Then the subject and commit message are wrong and need
to be updated to reflect your text.

This is a bugfix, not a cleanup.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/