Re: [PATCH net] staging: Remove set but not used variable ‘status’
On Sat, May 25, 2019 at 12:26:42PM +0800, Mao Wenan wrote: > Fixes gcc '-Wunused-but-set-variable' warning: > > drivers/staging/kpc2000/kpc_spi/spi_driver.c: In function > ‘kp_spi_transfer_one_message’: > drivers/staging/kpc2000/kpc_spi/spi_driver.c:282:9: warning: variable > ‘status’ set but not used [-Wunused-but-set-variable] > int status = 0; > ^~ > The variable 'status' is not used any more, remve it. > > Signed-off-by: Mao Wenan > --- > drivers/staging/kpc2000/kpc_spi/spi_driver.c | 3 --- > 1 file changed, 3 deletions(-) What is [PATCH net] in the subject for? This is not a networking driver :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: Remove set but not used variable ‘status’
On 2019/5/25 13:01, Greg KH wrote: > On Sat, May 25, 2019 at 12:26:42PM +0800, Mao Wenan wrote: >> Fixes gcc '-Wunused-but-set-variable' warning: >> >> drivers/staging/kpc2000/kpc_spi/spi_driver.c: In function >> ‘kp_spi_transfer_one_message’: >> drivers/staging/kpc2000/kpc_spi/spi_driver.c:282:9: warning: variable >> ‘status’ set but not used [-Wunused-but-set-variable] >> int status = 0; >> ^~ >> The variable 'status' is not used any more, remve it. >> >> Signed-off-by: Mao Wenan >> --- >> drivers/staging/kpc2000/kpc_spi/spi_driver.c | 3 --- >> 1 file changed, 3 deletions(-) > > What is [PATCH net] in the subject for? This is not a networking driver > :( Sorry, this is my mistake. > > > . > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: Remove set but not used variable ‘status’
On Sat, May 25, 2019 at 12:20 AM Mao Wenan wrote:
>
> The variable 'status' is not used any more, remve it.
> /* do the transfers for this message */
> list_for_each_entry(transfer, &m->transfers, transfer_list) {
> if (transfer->tx_buf == NULL && transfer->rx_buf == NULL &&
> transfer->len) {
> -status = -EINVAL;
> break;
> }
This looks like an error condition that's not reported to the spi core.
Instead of removing the status variable (which also removes the error value!),
maybe this should be reported to the spi core instead ?
Other spi drivers appear to do the following on the error path:
m->status = status;
return status;
>
> @@ -370,7 +368,6 @@ kp_spi_transfer_one_message(struct spi_master *master,
> struct spi_message *m)
>
> if (count != transfer->len) {
> -status = -EIO;
> break;
Same issue here.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net] staging: Remove set but not used variable ‘status’
On 2019/5/25 20:59, Sven Van Asbroeck wrote:
> On Sat, May 25, 2019 at 12:20 AM Mao Wenan wrote:
>>
>> The variable 'status' is not used any more, remve it.
>
>> /* do the transfers for this message */
>> list_for_each_entry(transfer, &m->transfers, transfer_list) {
>> if (transfer->tx_buf == NULL && transfer->rx_buf == NULL &&
>> transfer->len) {
>> -status = -EINVAL;
>> break;
>> }
>
> This looks like an error condition that's not reported to the spi core.
>
> Instead of removing the status variable (which also removes the error value!),
> maybe this should be reported to the spi core instead ?
>
> Other spi drivers appear to do the following on the error path:
> m->status = status;
> return status;
I have reviewed the code again, and it is good idea to store m->status in error
path, like below?
@@ -374,7 +374,7 @@ kp_spi_transfer_one_message(struct spi_master *master,
struct spi_message *m)
list_for_each_entry(transfer, &m->transfers, transfer_list) {
if (transfer->tx_buf == NULL && transfer->rx_buf == NULL &&
transfer->len) {
status = -EINVAL;
-break;
+goto error;
}
/* transfer */
@@ -412,7 +412,7 @@ kp_spi_transfer_one_message(struct spi_master *master,
struct spi_message *m)
if (count != transfer->len) {
status = -EIO;
-break;
+goto error;
}
}
...
/* done work */
spi_finalize_current_message(master);
return 0;
+
+ error:
+m->status = status;
+return status;
}
>
>>
>> @@ -370,7 +368,6 @@ kp_spi_transfer_one_message(struct spi_master *master,
>> struct spi_message *m)
>>
>> if (count != transfer->len) {
>> -status = -EIO;
>> break;
>
> Same issue here.
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
