All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] ses: Fix problems with simple enclosures
@ 2017-01-12 21:18 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2017-01-12 21:18 UTC (permalink / raw)
  To: James.Bottomley; +Cc: linux-scsi

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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2017-01-12 21:18 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-12 21:18 [bug report] ses: Fix problems with simple enclosures Dan Carpenter

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.