* [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work
@ 2019-05-31 18:53 ` Nathan Chancellor
0 siblings, 0 replies; 25+ messages in thread
From: Nathan Chancellor @ 2019-05-31 18:53 UTC (permalink / raw)
To: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux,
Nathan Chancellor
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 so that the atomic_set and dev_err statement don't
trigger for the cases that just break.
Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states")
Link: https://github.com/ClangBuiltLinux/linux/issues/502
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 727c31dc11a0..6714d8043e62 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -2118,7 +2118,7 @@ static unsigned long ibmvscsi_get_desired_dma(struct vio_dev *vdev)
static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata)
{
unsigned long flags;
- int rc;
+ int rc = 0;
char *action = "reset";
spin_lock_irqsave(hostdata->host->host_lock, flags);
--
2.22.0.rc2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-05-31 18:53 ` Nathan Chancellor 0 siblings, 0 replies; 25+ messages in thread From: Nathan Chancellor @ 2019-05-31 18:53 UTC (permalink / raw) To: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, Nathan Chancellor, linuxppc-dev, linux-kernel, linux-scsi 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 so that the atomic_set and dev_err statement don't trigger for the cases that just break. Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") Link: https://github.com/ClangBuiltLinux/linux/issues/502 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 727c31dc11a0..6714d8043e62 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -2118,7 +2118,7 @@ static unsigned long ibmvscsi_get_desired_dma(struct vio_dev *vdev) static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) { unsigned long flags; - int rc; + int rc = 0; char *action = "reset"; spin_lock_irqsave(hostdata->host->host_lock, flags); -- 2.22.0.rc2 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-05-31 18:53 ` Nathan Chancellor @ 2019-06-02 10:15 ` Michael Ellerman -1 siblings, 0 replies; 25+ messages in thread From: Michael Ellerman @ 2019-06-02 10:15 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, Nathan Chancellor, linuxppc-dev, linux-kernel, linux-scsi Hi Nathan, Nathan Chancellor <natechancellor@gmail.com> writes: > 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 so that the atomic_set and dev_err statement don't > trigger for the cases that just break. > > Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") > Link: https://github.com/ClangBuiltLinux/linux/issues/502 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 727c31dc11a0..6714d8043e62 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2118,7 +2118,7 @@ static unsigned long ibmvscsi_get_desired_dma(struct vio_dev *vdev) > static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) > { > unsigned long flags; > - int rc; > + int rc = 0; > char *action = "reset"; > > spin_lock_irqsave(hostdata->host->host_lock, flags); It's always preferable IMHO to keep any initialisation as localised as possible, so that the compiler can continue to warn about uninitialised usages elsewhere. In this case that would mean doing the rc = 0 in the switch, something like: diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 727c31dc11a0..7ee5755cf636 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -2123,9 +2123,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); @@ -2142,7 +2139,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; } But then that makes me wonder if that's actually correct? If we get an action that we don't recognise should we just throw it away like that? (by doing hostdata->action = IBMVSCSI_HOST_ACTION_NONE). Tyrel? cheers ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-02 10:15 ` Michael Ellerman 0 siblings, 0 replies; 25+ messages in thread From: Michael Ellerman @ 2019-06-02 10:15 UTC (permalink / raw) To: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, Nathan Chancellor, linuxppc-dev, linux-kernel, linux-scsi Hi Nathan, Nathan Chancellor <natechancellor@gmail.com> writes: > 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 so that the atomic_set and dev_err statement don't > trigger for the cases that just break. > > Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") > Link: https://github.com/ClangBuiltLinux/linux/issues/502 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 727c31dc11a0..6714d8043e62 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2118,7 +2118,7 @@ static unsigned long ibmvscsi_get_desired_dma(struct vio_dev *vdev) > static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) > { > unsigned long flags; > - int rc; > + int rc = 0; > char *action = "reset"; > > spin_lock_irqsave(hostdata->host->host_lock, flags); It's always preferable IMHO to keep any initialisation as localised as possible, so that the compiler can continue to warn about uninitialised usages elsewhere. In this case that would mean doing the rc = 0 in the switch, something like: diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 727c31dc11a0..7ee5755cf636 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -2123,9 +2123,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); @@ -2142,7 +2139,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; } But then that makes me wonder if that's actually correct? If we get an action that we don't recognise should we just throw it away like that? (by doing hostdata->action = IBMVSCSI_HOST_ACTION_NONE). Tyrel? cheers ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-02 10:15 ` Michael Ellerman @ 2019-06-03 3:23 ` Nathan Chancellor -1 siblings, 0 replies; 25+ messages in thread From: Nathan Chancellor @ 2019-06-03 3:23 UTC (permalink / raw) To: Michael Ellerman Cc: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen, clang-built-linux, linuxppc-dev, linux-kernel, linux-scsi Hi Michael, On Sun, Jun 02, 2019 at 08:15:38PM +1000, Michael Ellerman wrote: > Hi Nathan, > > It's always preferable IMHO to keep any initialisation as localised as > possible, so that the compiler can continue to warn about uninitialised > usages elsewhere. In this case that would mean doing the rc = 0 in the > switch, something like: I am certainly okay with implementing this in a v2. I mulled over which would be preferred, I suppose I guessed wrong :) Thank you for the review and input. > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 727c31dc11a0..7ee5755cf636 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2123,9 +2123,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); > @@ -2142,7 +2139,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; > } > > > But then that makes me wonder if that's actually correct? > > If we get an action that we don't recognise should we just throw it away > like that? (by doing hostdata->action = IBMVSCSI_HOST_ACTION_NONE). Tyrel? However, because of this, I will hold off on v2 until Tyrel can give some feedback. Thanks, Nathan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-03 3:23 ` Nathan Chancellor 0 siblings, 0 replies; 25+ messages in thread From: Nathan Chancellor @ 2019-06-03 3:23 UTC (permalink / raw) To: Michael Ellerman Cc: Tyrel Datwyler, linux-scsi, Martin K. Petersen, James E.J. Bottomley, linux-kernel, clang-built-linux, linuxppc-dev Hi Michael, On Sun, Jun 02, 2019 at 08:15:38PM +1000, Michael Ellerman wrote: > Hi Nathan, > > It's always preferable IMHO to keep any initialisation as localised as > possible, so that the compiler can continue to warn about uninitialised > usages elsewhere. In this case that would mean doing the rc = 0 in the > switch, something like: I am certainly okay with implementing this in a v2. I mulled over which would be preferred, I suppose I guessed wrong :) Thank you for the review and input. > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 727c31dc11a0..7ee5755cf636 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2123,9 +2123,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); > @@ -2142,7 +2139,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; > } > > > But then that makes me wonder if that's actually correct? > > If we get an action that we don't recognise should we just throw it away > like that? (by doing hostdata->action = IBMVSCSI_HOST_ACTION_NONE). Tyrel? However, because of this, I will hold off on v2 until Tyrel can give some feedback. Thanks, Nathan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-02 10:15 ` Michael Ellerman (?) (?) @ 2019-06-03 23:30 ` Tyrel Datwyler -1 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:30 UTC (permalink / raw) To: Michael Ellerman, Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, linuxppc-dev, linux-kernel, linux-scsi On 06/02/2019 03:15 AM, Michael Ellerman wrote: > Hi Nathan, > > Nathan Chancellor <natechancellor@gmail.com> writes: >> 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 so that the atomic_set and dev_err statement don't >> trigger for the cases that just break. >> >> Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") >> Link: https://github.com/ClangBuiltLinux/linux/issues/502 >> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> >> --- >> drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c >> index 727c31dc11a0..6714d8043e62 100644 >> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c >> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c >> @@ -2118,7 +2118,7 @@ static unsigned long ibmvscsi_get_desired_dma(struct vio_dev *vdev) >> static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) >> { >> unsigned long flags; >> - int rc; >> + int rc = 0; >> char *action = "reset"; >> >> spin_lock_irqsave(hostdata->host->host_lock, flags); > > It's always preferable IMHO to keep any initialisation as localised as > possible, so that the compiler can continue to warn about uninitialised > usages elsewhere. In this case that would mean doing the rc = 0 in the > switch, something like: > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 727c31dc11a0..7ee5755cf636 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2123,9 +2123,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); > @@ -2142,7 +2139,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; > } > > > But then that makes me wonder if that's actually correct? > > If we get an action that we don't recognise should we just throw it away > like that? (by doing hostdata->action = IBMVSCSI_HOST_ACTION_NONE). Tyrel? On initial pass I was ok with this, but after thinking on it I think it is more subtle. The right approach is to set rc = 0 for HOST_ACTION_UNBLOCK as we want to fall through. For HOST_ACTION_NONE and default we need to return directly from the function. -Tyrel > > cheers > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-05-31 18:53 ` Nathan Chancellor (?) (?) @ 2019-06-03 8:45 ` Christophe Leroy -1 siblings, 0 replies; 25+ messages in thread From: Christophe Leroy @ 2019-06-03 8:45 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, linuxppc-dev, linux-kernel, linux-scsi Le 31/05/2019 à 20:53, Nathan Chancellor a écrit : > 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 so that the atomic_set and dev_err statement don't > trigger for the cases that just break. > > Fixes: 035a3c4046b5 ("scsi: ibmvscsi: redo driver work thread to use enum action states") > Link: https://github.com/ClangBuiltLinux/linux/issues/502 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c > index 727c31dc11a0..6714d8043e62 100644 > --- a/drivers/scsi/ibmvscsi/ibmvscsi.c > +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c > @@ -2118,7 +2118,7 @@ static unsigned long ibmvscsi_get_desired_dma(struct vio_dev *vdev) > static void ibmvscsi_do_work(struct ibmvscsi_host_data *hostdata) > { > unsigned long flags; > - int rc; > + int rc = 0; I don't think the above is the best solution, as it hides the warning instead of really fixing it. Your problem is that some legs of the switch are missing setting the value of rc, it would therefore be better to fix the legs instead of setting a default value which may not be correct for every case, allthough it may be at the time being. Christophe > char *action = "reset"; > > spin_lock_irqsave(hostdata->host->host_lock, flags); > ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-05-31 18:53 ` Nathan Chancellor @ 2019-06-03 22:19 ` Nathan Chancellor -1 siblings, 0 replies; 25+ messages in thread From: Nathan Chancellor @ 2019-06-03 22:19 UTC (permalink / raw) To: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux, Nathan Chancellor, Michael Ellerman 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(-) 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; } -- 2.22.0.rc3 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-03 22:19 ` Nathan Chancellor 0 siblings, 0 replies; 25+ messages in thread From: Nathan Chancellor @ 2019-06-03 22:19 UTC (permalink / raw) To: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linux-kernel, clang-built-linux, Nathan Chancellor, linuxppc-dev 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(-) 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; } -- 2.22.0.rc3 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-03 22:19 ` Nathan Chancellor @ 2019-06-03 23:25 ` Tyrel Datwyler -1 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:25 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux, Michael Ellerman 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> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-03 23:25 ` Tyrel Datwyler 0 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:25 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, linuxppc-dev, linux-kernel, linux-scsi 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> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-03 23:25 ` Tyrel Datwyler @ 2019-06-03 23:34 ` Tyrel Datwyler -1 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:34 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux, Michael Ellerman 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; ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-03 23:34 ` Tyrel Datwyler 0 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:34 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, linuxppc-dev, linux-kernel, linux-scsi 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; ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-03 23:34 ` Tyrel Datwyler @ 2019-06-03 23:35 ` Tyrel Datwyler -1 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:35 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux, Michael Ellerman 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; > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-03 23:35 ` Tyrel Datwyler 0 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:35 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, linuxppc-dev, linux-kernel, linux-scsi 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; > ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-03 22:19 ` Nathan Chancellor @ 2019-06-03 23:44 ` Nathan Chancellor -1 siblings, 0 replies; 25+ messages in thread From: Nathan Chancellor @ 2019-06-03 23:44 UTC (permalink / raw) To: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux, Nathan Chancellor, Michael Ellerman 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 in the IBMVSCSI_HOST_ACTION_UNBLOCK case statement then shuffle IBMVSCSI_HOST_ACTION_NONE down to the default case statement and make it return early so that rc is never used uninitialized in this function. 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> Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com> 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. v2 -> v3: * default and IBMVSCSI_HOST_ACTION_NONE now return early from the function, as requested by Tyrel. drivers/scsi/ibmvscsi/ibmvscsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 65053daef5f7..7f66a7783209 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -2109,8 +2109,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); @@ -2128,8 +2128,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: default: - break; + spin_unlock_irqrestore(hostdata->host->host_lock, flags); + return; } hostdata->action = IBMVSCSI_HOST_ACTION_NONE; -- 2.22.0.rc3 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-03 23:44 ` Nathan Chancellor 0 siblings, 0 replies; 25+ messages in thread From: Nathan Chancellor @ 2019-06-03 23:44 UTC (permalink / raw) To: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linux-kernel, clang-built-linux, Nathan Chancellor, linuxppc-dev 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 in the IBMVSCSI_HOST_ACTION_UNBLOCK case statement then shuffle IBMVSCSI_HOST_ACTION_NONE down to the default case statement and make it return early so that rc is never used uninitialized in this function. 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> Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com> 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. v2 -> v3: * default and IBMVSCSI_HOST_ACTION_NONE now return early from the function, as requested by Tyrel. drivers/scsi/ibmvscsi/ibmvscsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 65053daef5f7..7f66a7783209 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -2109,8 +2109,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); @@ -2128,8 +2128,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: default: - break; + spin_unlock_irqrestore(hostdata->host->host_lock, flags); + return; } hostdata->action = IBMVSCSI_HOST_ACTION_NONE; -- 2.22.0.rc3 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-03 23:44 ` Nathan Chancellor @ 2019-06-03 23:58 ` Tyrel Datwyler -1 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:58 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linux-kernel, clang-built-linux, linuxppc-dev On 06/03/2019 04:44 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 in the IBMVSCSI_HOST_ACTION_UNBLOCK case statement then > shuffle IBMVSCSI_HOST_ACTION_NONE down to the default case statement and > make it return early so that rc is never used uninitialized in this > function. > > 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> > Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-03 23:58 ` Tyrel Datwyler 0 siblings, 0 replies; 25+ messages in thread From: Tyrel Datwyler @ 2019-06-03 23:58 UTC (permalink / raw) To: Nathan Chancellor, Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen Cc: clang-built-linux, linuxppc-dev, linux-kernel, linux-scsi On 06/03/2019 04:44 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 in the IBMVSCSI_HOST_ACTION_UNBLOCK case statement then > shuffle IBMVSCSI_HOST_ACTION_NONE down to the default case statement and > make it return early so that rc is never used uninitialized in this > function. > > 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> > Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-03 23:44 ` Nathan Chancellor @ 2019-06-05 2:39 ` Martin K. Petersen -1 siblings, 0 replies; 25+ messages in thread From: Martin K. Petersen @ 2019-06-05 2:39 UTC (permalink / raw) To: Nathan Chancellor Cc: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen, linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux, Michael Ellerman Nathan, > 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: > ^~~~~~~~~~~~~~~~~~~~~~~~~ Applied to 5.3/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-05 2:39 ` Martin K. Petersen 0 siblings, 0 replies; 25+ messages in thread From: Martin K. Petersen @ 2019-06-05 2:39 UTC (permalink / raw) To: Nathan Chancellor Cc: Tyrel Datwyler, Martin K. Petersen, linux-scsi, James E.J. Bottomley, linux-kernel, clang-built-linux, linuxppc-dev Nathan, > 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: > ^~~~~~~~~~~~~~~~~~~~~~~~~ Applied to 5.3/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work 2019-06-05 2:39 ` Martin K. Petersen (?) @ 2019-06-05 11:08 ` Michael Ellerman -1 siblings, 0 replies; 25+ messages in thread From: Michael Ellerman @ 2019-06-05 11:08 UTC (permalink / raw) To: Martin K. Petersen, Nathan Chancellor Cc: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen, linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux "Martin K. Petersen" <martin.petersen@oracle.com> writes: > Nathan, > >> 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: >> ^~~~~~~~~~~~~~~~~~~~~~~~~ > > Applied to 5.3/scsi-queue, thanks! Thanks all. cheers ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-05 11:08 ` Michael Ellerman 0 siblings, 0 replies; 25+ messages in thread From: Michael Ellerman @ 2019-06-05 11:08 UTC (permalink / raw) To: Martin K. Petersen, Nathan Chancellor Cc: Tyrel Datwyler, Martin K. Petersen, linux-scsi, James E.J. Bottomley, linux-kernel, clang-built-linux, linuxppc-dev "Martin K. Petersen" <martin.petersen@oracle.com> writes: > Nathan, > >> 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: >> ^~~~~~~~~~~~~~~~~~~~~~~~~ > > Applied to 5.3/scsi-queue, thanks! Thanks all. cheers ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work @ 2019-06-05 11:08 ` Michael Ellerman 0 siblings, 0 replies; 25+ messages in thread From: Michael Ellerman @ 2019-06-05 11:08 UTC (permalink / raw) To: Nathan Chancellor Cc: Tyrel Datwyler, James E.J. Bottomley, Martin K. Petersen, linux-scsi, linuxppc-dev, linux-kernel, clang-built-linux "Martin K. Petersen" <martin.petersen@oracle.com> writes: > Nathan, > >> 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: >> ^~~~~~~~~~~~~~~~~~~~~~~~~ > > Applied to 5.3/scsi-queue, thanks! Thanks all. cheers ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2019-06-05 11:09 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-31 18:53 [PATCH] scsi: ibmvscsi: Don't use rc uninitialized in ibmvscsi_do_work Nathan Chancellor 2019-05-31 18:53 ` Nathan Chancellor 2019-06-02 10:15 ` Michael Ellerman 2019-06-02 10:15 ` Michael Ellerman 2019-06-03 3:23 ` Nathan Chancellor 2019-06-03 3:23 ` Nathan Chancellor 2019-06-03 23:30 ` Tyrel Datwyler 2019-06-03 8:45 ` Christophe Leroy 2019-06-03 22:19 ` [PATCH v2] " Nathan Chancellor 2019-06-03 22:19 ` Nathan Chancellor 2019-06-03 23:25 ` Tyrel Datwyler 2019-06-03 23:25 ` Tyrel Datwyler 2019-06-03 23:34 ` Tyrel Datwyler 2019-06-03 23:34 ` Tyrel Datwyler 2019-06-03 23:35 ` Tyrel Datwyler 2019-06-03 23:35 ` Tyrel Datwyler 2019-06-03 23:44 ` [PATCH v3] " Nathan Chancellor 2019-06-03 23:44 ` Nathan Chancellor 2019-06-03 23:58 ` Tyrel Datwyler 2019-06-03 23:58 ` Tyrel Datwyler 2019-06-05 2:39 ` Martin K. Petersen 2019-06-05 2:39 ` Martin K. Petersen 2019-06-05 11:08 ` Michael Ellerman 2019-06-05 11:08 ` Michael Ellerman 2019-06-05 11:08 ` Michael Ellerman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.