* [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once @ 2014-06-25 14:04 Rickard Strandqvist 2014-06-25 14:26 ` Maurizio Lombardi 0 siblings, 1 reply; 5+ messages in thread From: Rickard Strandqvist @ 2014-06-25 14:04 UTC (permalink / raw) To: Eddie Wai, James E.J. Bottomley Cc: Rickard Strandqvist, linux-scsi, linux-kernel 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_strandqvist@spectrumdigital.se> --- drivers/scsi/bnx2i/bnx2i_iscsi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index 166543f..fdf7bc3 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -1643,7 +1643,6 @@ 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; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once 2014-06-25 14:04 [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once Rickard Strandqvist @ 2014-06-25 14:26 ` Maurizio Lombardi 2014-06-25 17:13 ` Eddie Wai 0 siblings, 1 reply; 5+ messages in thread From: Maurizio Lombardi @ 2014-06-25 14:26 UTC (permalink / raw) To: Rickard Strandqvist, Eddie Wai, James E.J. Bottomley Cc: linux-scsi, linux-kernel Hi, On 06/25/2014 04:04 PM, Rickard Strandqvist wrote: > 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_strandqvist@spectrumdigital.se> > --- > drivers/scsi/bnx2i/bnx2i_iscsi.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c > index 166543f..fdf7bc3 100644 > --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c > +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c > @@ -1643,7 +1643,6 @@ 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; > Eddie, The code modifies the content of stats->custom[2], so shouldn't custom_length be set to 3? Why is it set to zero at the end of this function? Regards, Maurizio Lombardi ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once 2014-06-25 14:26 ` Maurizio Lombardi @ 2014-06-25 17:13 ` Eddie Wai 2014-06-26 0:28 ` Rickard Strandqvist 0 siblings, 1 reply; 5+ messages in thread From: Eddie Wai @ 2014-06-25 17:13 UTC (permalink / raw) To: Maurizio Lombardi Cc: Rickard Strandqvist, James E.J. Bottomley, linux-scsi, linux-kernel On Wed, 2014-06-25 at 16:26 +0200, Maurizio Lombardi wrote: > Hi, > > On 06/25/2014 04:04 PM, Rickard Strandqvist wrote: > > 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_strandqvist@spectrumdigital.se> > > --- > > drivers/scsi/bnx2i/bnx2i_iscsi.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c > > index 166543f..fdf7bc3 100644 > > --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c > > +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c > > @@ -1643,7 +1643,6 @@ 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; > > > > Eddie, > > The code modifies the content of stats->custom[2], so shouldn't custom_length be set to 3? > Why is it set to zero at the end of this function? Nice find. This is literally a day1 bug. Yes, I agree that the custom_length should be left at 3. Otherwise, the nlmsg replied back to the application would not have the custom message. Thanks. > > Regards, > Maurizio Lombardi ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once 2014-06-25 17:13 ` Eddie Wai @ 2014-06-26 0:28 ` Rickard Strandqvist 2014-06-26 8:04 ` Maurizio Lombardi 0 siblings, 1 reply; 5+ messages in thread From: Rickard Strandqvist @ 2014-06-26 0:28 UTC (permalink / raw) To: Eddie Wai Cc: Maurizio Lombardi, James E.J. Bottomley, linux-scsi, linux-kernel@vger.kernel.org 2014-06-25 19:13 GMT+02:00 Eddie Wai <eddie.wai@broadcom.com>: > On Wed, 2014-06-25 at 16:26 +0200, Maurizio Lombardi wrote: >> Hi, >> >> On 06/25/2014 04:04 PM, Rickard Strandqvist wrote: >> > 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_strandqvist@spectrumdigital.se> >> > --- >> > drivers/scsi/bnx2i/bnx2i_iscsi.c | 1 - >> > 1 file changed, 1 deletion(-) >> > >> > diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c >> > index 166543f..fdf7bc3 100644 >> > --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c >> > +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c >> > @@ -1643,7 +1643,6 @@ 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; >> > >> >> Eddie, >> >> The code modifies the content of stats->custom[2], so shouldn't custom_length be set to 3? >> Why is it set to zero at the end of this function? > Nice find. This is literally a day1 bug. Yes, I agree that the > custom_length should be left at 3. Otherwise, the nlmsg replied back to > the application would not have the custom message. Thanks. >> >> Regards, >> Maurizio Lombardi > > Hi, and thank you! If it's not obvious, I do all my fixes without changing the previous intent. But obviously it is not always right, rather it is one of main reasons to fix this type of error :) But I'll make a new patch then, with = 3 ? Kind regards Rickard Strandqvist ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once 2014-06-26 0:28 ` Rickard Strandqvist @ 2014-06-26 8:04 ` Maurizio Lombardi 0 siblings, 0 replies; 5+ messages in thread From: Maurizio Lombardi @ 2014-06-26 8:04 UTC (permalink / raw) To: Rickard Strandqvist, Eddie Wai Cc: James E.J. Bottomley, linux-scsi, linux-kernel@vger.kernel.org Hi, On 06/26/2014 02:28 AM, Rickard Strandqvist wrote: > > If it's not obvious, I do all my fixes without changing the previous > intent. But obviously it is not always right, rather it is one of main > reasons to fix this type of error :) Yes it is obvious, your patch was correct (it didn't modify the function behaviour) but helped to spot a defect. > > But I'll make a new patch then, with = 3 ? Yes, please submit a new patch which sets custom_length = 3 at the end of the function. Thanks, Maurizio Lombardi ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-06-26 8:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-06-25 14:04 [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once Rickard Strandqvist 2014-06-25 14:26 ` Maurizio Lombardi 2014-06-25 17:13 ` Eddie Wai 2014-06-26 0:28 ` Rickard Strandqvist 2014-06-26 8:04 ` Maurizio Lombardi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox