Re: [PATCH] scsi: ses: Handle positive SCSI error from ses_recv_diag()

[Greg Kroah-Hartman]

On Mon, Feb 23, 2026 at 04:44:59PM +0100, Greg Kroah-Hartman wrote:
> ses_recv_diag() can return a positive value, which also means that an
> error happened, so do not only test for negative values.
> 
> Cc: "James E.J. Bottomley" <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> Cc: "Martin K. Petersen" <martin.petersen@xxxxxxxxxx>
> Cc: stable <stable@xxxxxxxxxx>
> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/scsi/ses.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index 35101e9b7ba7..128042c734cc 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -215,7 +215,7 @@ static unsigned char *ses_get_page2_descriptor(struct enclosure_device *edev,
>  	unsigned char *type_ptr = ses_dev->page1_types;
>  	unsigned char *desc_ptr = ses_dev->page2 + 8;
>  
> -	if (ses_recv_diag(sdev, 2, ses_dev->page2, ses_dev->page2_len) < 0)
> +	if (ses_recv_diag(sdev, 2, ses_dev->page2, ses_dev->page2_len))
>  		return NULL;
>  
>  	for (i = 0; i < ses_dev->page1_num_types; i++, type_ptr += 4) {
> -- 
> 2.53.0
> 

Along these lines, any specific reason why the following code in
ses_enclosure_data_process() doesn't also check the return value of
ses_recv_diag():

	/* re-read page 10 */
	if (ses_dev->page10)
		ses_recv_diag(sdev, 10, ses_dev->page10, ses_dev->page10_len);
	/* Page 7 for the descriptors is optional */
	result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
	if (result)
		goto simple_populate;

Is that because re-reading always succeeds, or because it can fail and
we just want to ignore it?  It feels odd that the "optional" read
command for page 7 is checked by not page 10.  But hey, scsi is odd :)

thank,s

greg k-h