| Message ID | 20190603221941.65432-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch next (a3bf9fbdad600b1e4335dd90979f8d6072e4f602) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
On 06/03/2019 03:19 PM, Nathan Chancellor wrote: > clang warns: > > drivers/scsi/ibmvscsi/ibmvscsi.c:2126:7: warning: variable 'rc' is used > uninitialized whenever switch case is taken [-Wsometimes-uninitialized] > case IBMVSCSI_HOST_ACTION_NONE: > ^~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/scsi/ibmvscsi/ibmvscsi.c:2151:6: note: uninitialized use occurs > here > if (rc) { > ^~ > > Initialize rc to zero in the case statements that clang mentions so that > the atomic_set and dev_err statement don't trigger for them. > > Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") > Link: https://github.com/ClangBuiltLinux/linux/issues/502 > Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com>
On 06/03/2019 04:25 PM, Tyrel Datwyler wrote: > On 06/03/2019 03:19 PM, Nathan Chancellor wrote: >> clang warns: >> >> drivers/scsi/ibmvscsi/ibmvscsi.c:2126:7: warning: variable 'rc' is used >> uninitialized whenever switch case is taken [-Wsometimes-uninitialized] >> case IBMVSCSI_HOST_ACTION_NONE: >> ^~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/scsi/ibmvscsi/ibmvscsi.c:2151:6: note: uninitialized use occurs >> here >> if (rc) { >> ^~ >> >> Initialize rc to zero in the case statements that clang mentions so that >> the atomic_set and dev_err statement don't trigger for them. >> >> Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") >> Link: https://github.com/ClangBuiltLinux/linux/issues/502 >> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com> > On second thought NACK. See my response to Michael earlier in the thread. I think this is the better solution: diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 727c31dc11a0..c3cf05dd8733 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -2123,8 +2123,8 @@ static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) spin_lock_irqsave(hostdata->host->host_lock, flags); switch (hostdata->action) { - case IBMVSCSI_HOST_ACTION_NONE: case IBMVSCSI_HOST_ACTION_UNBLOCK: + rc = 0; break; case IBMVSCSI_HOST_ACTION_RESET: spin_unlock_irqrestore(hostdata->host->host_lock, flags); @@ -2142,8 +2142,9 @@ static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) if (!rc) rc = ibmvscsi_send_crq(hostdata, 0xC001000000000000LL, 0); break; + case IBMVSCSI_HOST_ACTION_NONE: default: - break; + return; } hostdata->action = IBMVSCSI_HOST_ACTION_NONE;
On 06/03/2019 04:34 PM, Tyrel Datwyler wrote: > On 06/03/2019 04:25 PM, Tyrel Datwyler wrote: >> On 06/03/2019 03:19 PM, Nathan Chancellor wrote: >>> clang warns: >>> >>> drivers/scsi/ibmvscsi/ibmvscsi.c:2126:7: warning: variable 'rc' is used >>> uninitialized whenever switch case is taken [-Wsometimes-uninitialized] >>> case IBMVSCSI_HOST_ACTION_NONE: >>> ^~~~~~~~~~~~~~~~~~~~~~~~~ >>> drivers/scsi/ibmvscsi/ibmvscsi.c:2151:6: note: uninitialized use occurs >>> here >>> if (rc) { >>> ^~ >>> >>> Initialize rc to zero in the case statements that clang mentions so that >>> the atomic_set and dev_err statement don't trigger for them. >>> >>> Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") >>> Link: https://github.com/ClangBuiltLinux/linux/issues/502 >>> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> >>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> >> >> Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com> >> > > On second thought NACK. See my response to Michael earlier in the thread. > > I think this is the better solution: > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 727c31dc11a0..c3cf05dd8733 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2123,8 +2123,8 @@ static void ibmvscsi_do_work(struct ibmvscsi_host_data > *hostdata) > > spin_lock_irqsave(hostdata->host->host_lock, flags); > switch (hostdata->action) { > - case IBMVSCSI_HOST_ACTION_NONE: > case IBMVSCSI_HOST_ACTION_UNBLOCK: > + rc = 0; > break; > case IBMVSCSI_HOST_ACTION_RESET: > spin_unlock_irqrestore(hostdata->host->host_lock, flags); > @@ -2142,8 +2142,9 @@ static void ibmvscsi_do_work(struct ibmvscsi_host_data > *hostdata) > if (!rc) > rc = ibmvscsi_send_crq(hostdata, 0xC001000000000000LL, 0); > break; > + case IBMVSCSI_HOST_ACTION_NONE: > default: > - break; Need a spin_unlock_irqrestore() here before the return. -Tyrel > + return; > } > > hostdata->action = IBMVSCSI_HOST_ACTION_NONE; >
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 65053daef5f7..3b5647d622d9 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -2109,9 +2109,6 @@ static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) spin_lock_irqsave(hostdata->host->host_lock, flags); switch (hostdata->action) { - case IBMVSCSI_HOST_ACTION_NONE: - case IBMVSCSI_HOST_ACTION_UNBLOCK: - break; case IBMVSCSI_HOST_ACTION_RESET: spin_unlock_irqrestore(hostdata->host->host_lock, flags); rc = ibmvscsi_reset_crq_queue(&hostdata->queue, hostdata); @@ -2128,7 +2125,10 @@ static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) if (!rc) rc = ibmvscsi_send_crq(hostdata, 0xC001000000000000LL, 0); break; + case IBMVSCSI_HOST_ACTION_NONE: + case IBMVSCSI_HOST_ACTION_UNBLOCK: default: + rc = 0; break; }
clang warns: drivers/scsi/ibmvscsi/ibmvscsi.c:2126:7: warning: variable 'rc' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized] case IBMVSCSI_HOST_ACTION_NONE: ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/scsi/ibmvscsi/ibmvscsi.c:2151:6: note: uninitialized use occurs here if (rc) { ^~ Initialize rc to zero in the case statements that clang mentions so that the atomic_set and dev_err statement don't trigger for them. Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") Link: https://github.com/ClangBuiltLinux/linux/issues/502 Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- v1 -> v2: * Initialize rc in the case statements, rather than at the top of the function, as suggested by Michael. drivers/scsi/ibmvscsi/ibmvscsi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)