All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.