Re: [PATCH 18/24] st: return error code in st_scsi_execute()

[Hannes Reinecke <hare@suse.de>]

On 10/21/19 6:41 PM, Bart Van Assche wrote:
> On 10/21/19 2:53 AM, Hannes Reinecke wrote:
>> We should return the actual error code in st_scsi_execute(),
>> avoiding the need to use DRIVER_ERROR.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/scsi/st.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
>> index e3266a64a477..5f38369cc62f 100644
>> --- a/drivers/scsi/st.c
>> +++ b/drivers/scsi/st.c
>> @@ -549,7 +549,7 @@ static int st_scsi_execute(struct st_request
>> *SRpnt, const unsigned char *cmd,
>>               data_direction == DMA_TO_DEVICE ?
>>               REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, 0);
>>       if (IS_ERR(req))
>> -        return DRIVER_ERROR << 24;
>> +        return PTR_ERR(req);
>>       rq = scsi_req(req);
>>       req->rq_flags |= RQF_QUIET;
>>   @@ -560,7 +560,7 @@ static int st_scsi_execute(struct st_request
>> *SRpnt, const unsigned char *cmd,
>>                         GFP_KERNEL);
>>           if (err) {
>>               blk_put_request(req);
>> -            return DRIVER_ERROR << 24;
>> +            return err;
>>           }
>>       }
> 
> The patch description looks confusing to me. Is it perhaps because the
> caller compares the st_scsi_execute() return value with zero and doesn't
> use the return value in any other way that it is fine to return an
> integer error code instead of a SCSI status?
> 
Yes. The caller does:

	ret = st_scsi_execute(SRpnt, cmd, direction, NULL, bytes, timeout,
			      retries);
	if (ret) {
		/* could not allocate the buffer or request was too large */
		(STp->buffer)->syscall_result = (-EBUSY);
		(STp->buffer)->last_SRpnt = NULL;

So it's immaterial _what_ we return here as long as it's non-zero.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      Teamlead Storage & Networking
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer