Re: [PATCH] scsi: Change the return type of the .queuecommand() callback

[John Garry <john.g.garry@oracle.com>]

On 06/01/2026 18:52, Bart Van Assche wrote:
> Let the compiler verify whether a valid value is returned by the
> .queuecommand() implementations by changing their return type from 'int'
> into 'enum scsi_qc_status'. No functionality has been changed.
> 
> Cc: Damien Le Moal <dlemoal@kernel.org>
> Cc: John Garry <john.g.garry@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---

Using enum scsi_qc_status over an int makes the code less condense... 
scsi_qc_sts might have been a better name, although I do realise that 
this was introduced in another change.

BTW, are there any drivers which were not passing back a scsi_qc_status 
type? I assume not.

On 06/01/2026 18:52, Bart Van Assche wrote:
 > +static enum scsi_qc_status sbp2_scsi_queuecommand(struct Scsi_Host 
*shost,
 > +						  struct scsi_cmnd *cmd)
 >   {
 >   	struct sbp2_logical_unit *lu = cmd->device->hostdata;
 >   	struct fw_device *device = target_parent_device(lu->tgt);
 >   	struct sbp2_command_orb *orb;
 > -	int generation, retval = SCSI_MLQUEUE_HOST_BUSY;
 > +	int generation;
 > +	enum scsi_qc_status retval = SCSI_MLQUEUE_HOST_BUSY;

I suggest trying to maintain the reverse fir tree style, when possible. 
I do realise that it is not the case here, but it was close.


> index e87cf7eadd26..41ba6e7e799a 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -84,13 +84,15 @@ struct scsi_host_template {
>   	 *
>   	 * STATUS: REQUIRED
>   	 */
> -	int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
> +	enum scsi_qc_status (* queuecommand)(struct Scsi_Host *,

please remove unnecessary whitespace before "queuecommand"

> +					     struct scsi_cmnd *);
>   
>   	/*

On 06/01/2026 18:52, Bart Van Assche wrote:
 >   #define DEF_SCSI_QCMD(func_name) \
 > -	int func_name(struct Scsi_Host *shost, struct scsi_cmnd *cmd)	\
 > +	enum scsi_qc_status func_name(struct Scsi_Host *shost,		\
 > +				      struct scsi_cmnd *cmd)		\
 >   	{								\
 >   		unsigned long irq_flags;				\
 >   		int rc;		

why not an enum scsi_qc_status?

Note I have not fully gone through the change in the patch.