diff mbox series

[04/23] targe/pscsi: fix the warning in pscsi_complete_cmd

Message ID 20210228055645.22253-5-chaitanya.kulkarni@wdc.com (mailing list archive)
State Accepted
Headers show
Series target: code cleanup | expand

Commit Message

Chaitanya Kulkarni Feb. 28, 2021, 5:56 a.m. UTC
This fixes the compilation warning in pscsi_complete_cmd():-

    drivers/target/target_core_pscsi.c: In function ‘pscsi_complete_cmd’:
    drivers/target/target_core_pscsi.c:624:5: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
         ; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */
         ^

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/target/target_core_pscsi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Johannes Thumshirn March 1, 2021, 12:30 p.m. UTC | #1
On 28/02/2021 07:00, Chaitanya Kulkarni wrote:
> This fixes the compilation warning in pscsi_complete_cmd():-
> 
>     drivers/target/target_core_pscsi.c: In function ‘pscsi_complete_cmd’:
>     drivers/target/target_core_pscsi.c:624:5: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
>          ; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */
>          ^
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  drivers/target/target_core_pscsi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
> index 3cbc074992bc..689e503e3301 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -620,8 +620,9 @@ static void pscsi_complete_cmd(struct se_cmd *cmd, u8 scsi_status,
>  			unsigned char *buf;
>  
>  			buf = transport_kmap_data_sg(cmd);
> -			if (!buf)
> +			if (!buf) {
>  				; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */
> +			}
>  
>  			if (cdb[0] == MODE_SENSE_10) {
>  				if (!(buf[3] & 0x80))
> 

Do you have any plans to actually address that XXX? Because if
transport_kmap_data_sg() fails (and it can fail for various reasons)
and we get a MODE_SENSE_10 we'll trip over a NULL pointer.

I know this is outside of the scope of this patchset, but would be nice anyways.
Chaitanya Kulkarni March 2, 2021, 3:57 a.m. UTC | #2
On 3/1/21 04:30, Johannes Thumshirn wrote:
>> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
>> index 3cbc074992bc..689e503e3301 100644
>> --- a/drivers/target/target_core_pscsi.c
>> +++ b/drivers/target/target_core_pscsi.c
>> @@ -620,8 +620,9 @@ static void pscsi_complete_cmd(struct se_cmd *cmd, u8 scsi_status,
>>  			unsigned char *buf;
>>  
>>  			buf = transport_kmap_data_sg(cmd);
>> -			if (!buf)
>> +			if (!buf) {
>>  				; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */
>> +			}
>>  
>>  			if (cdb[0] == MODE_SENSE_10) {
>>  				if (!(buf[3] & 0x80))
>>
> Do you have any plans to actually address that XXX? Because if
> transport_kmap_data_sg() fails (and it can fail for various reasons)
> and we get a MODE_SENSE_10 we'll trip over a NULL pointer.
>
> I know this is outside of the scope of this patchset, but would be nice anyways.
>
Yes we I can try and fix that, but surprisingly parse cmd return type is
void.
It maybe a bigger change will keep it separate. Thanks for the reviews.
diff mbox series

Patch

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 3cbc074992bc..689e503e3301 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -620,8 +620,9 @@  static void pscsi_complete_cmd(struct se_cmd *cmd, u8 scsi_status,
 			unsigned char *buf;
 
 			buf = transport_kmap_data_sg(cmd);
-			if (!buf)
+			if (!buf) {
 				; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */
+			}
 
 			if (cdb[0] == MODE_SENSE_10) {
 				if (!(buf[3] & 0x80))