public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: James.Bottomley@HansenPartnership.com
Cc: linux-scsi@vger.kernel.org
Subject: [bug report] ses: Fix problems with simple enclosures
Date: Fri, 13 Jan 2017 00:18:36 +0300	[thread overview]
Message-ID: <20170112211836.GA5459@mwanda> (raw)

Hello James Bottomley,

The patch 3417c1b5cb1f: "ses: Fix problems with simple enclosures"
from Dec 8, 2015, leads to the following static checker warning:

	drivers/scsi/ses.c:103 ses_recv_diag()
	info: return a literal instead of 'ret'

drivers/scsi/ses.c
    86  static int ses_recv_diag(struct scsi_device *sdev, int page_code,
    87                           void *buf, int bufflen)
    88  {
    89          int ret;
    90          unsigned char cmd[] = {
    91                  RECEIVE_DIAGNOSTIC,
    92                  1,              /* Set PCV bit */
    93                  page_code,
    94                  bufflen >> 8,
    95                  bufflen & 0xff,
    96                  0
    97          };
    98          unsigned char recv_page_code;
    99  
   100          ret =  scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
   101                                  NULL, SES_TIMEOUT, SES_RETRIES, NULL);
   102          if (unlikely(!ret))
   103                  return ret;

This code works, but why is it unlikely() that scsi_execute_req() will
succeed?

   104  
   105          recv_page_code = ((unsigned char *)buf)[0];
   106  
   107          if (likely(recv_page_code == page_code))
   108                  return ret;
   109  
   110          /* successful diagnostic but wrong page code.  This happens to some
   111           * USB devices, just print a message and pretend there was an error */
   112  
   113          sdev_printk(KERN_ERR, sdev,
   114                      "Wrong diagnostic page; asked for %d got %u\n",
   115                      page_code, recv_page_code);
   116  
   117          return -EINVAL;

We sort of mixing a bunch of different types of error codes here aren't
we?  Shouldn't this be "return DRIVER_ERROR << 24;" like how
scsi_execute_req() does it?  I don't think the callers care.

   118  }

regards,
dan carpenter

                 reply	other threads:[~2017-01-12 21:18 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170112211836.GA5459@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox