Remove unnesessary casts from void*.
Style fixes with assignments in if (...).
Signed-off-by: Kulikov Vasiliy <[email protected]>
---
drivers/scsi/sg.c | 63 +++++++++++++++++++++++++++++++++++++----------------
1 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index ef752b2..58215af 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -210,7 +210,7 @@ static void sg_put_dev(Sg_device *sdp);
static int sg_allow_access(struct file *filp, unsigned char *cmd)
{
- struct sg_fd *sfp = (struct sg_fd *)filp->private_data;
+ struct sg_fd *sfp = filp->private_data;
if (sfp->parentdp->device->type == TYPE_SCANNER)
return 0;
@@ -315,11 +315,15 @@ sg_put:
static int
sg_release(struct inode *inode, struct file *filp)
{
+ Sg_fd *sfp = filp->private_data;
Sg_device *sdp;
- Sg_fd *sfp;
- if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
+ if (!sfp)
+ return -ENXIO;
+ sdp = sfp->parentdp;
+ if (!sdp)
return -ENXIO;
+
SCSI_LOG_TIMEOUT(3, printk("sg_release: %s\n", sdp->disk->disk_name));
sfp->closed = 1;
@@ -334,16 +338,20 @@ sg_release(struct inode *inode, struct file *filp)
static ssize_t
sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
{
+ Sg_fd *sfp = filp->private_data;
Sg_device *sdp;
- Sg_fd *sfp;
Sg_request *srp;
int req_pack_id = -1;
sg_io_hdr_t *hp;
struct sg_header *old_hdr = NULL;
int retval = 0;
- if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
+ if (!sfp)
+ return -ENXIO;
+ sdp = sfp->parentdp;
+ if (!sdp)
return -ENXIO;
+
SCSI_LOG_TIMEOUT(3, printk("sg_read: %s, count=%d\n",
sdp->disk->disk_name, (int) count));
@@ -526,15 +534,19 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
int mxsize, cmd_size, k;
int input_size, blocking;
unsigned char opcode;
+ Sg_fd *sfp = filp->private_data;
Sg_device *sdp;
- Sg_fd *sfp;
Sg_request *srp;
struct sg_header old_hdr;
sg_io_hdr_t *hp;
unsigned char cmnd[MAX_COMMAND_SIZE];
- if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
+ if (!sfp)
+ return -ENXIO;
+ sdp = sfp->parentdp;
+ if (!sdp)
return -ENXIO;
+
SCSI_LOG_TIMEOUT(3, printk("sg_write: %s, count=%d\n",
sdp->disk->disk_name, (int) count));
if (sdp->detached)
@@ -763,12 +775,15 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
void __user *p = (void __user *)arg;
int __user *ip = p;
int result, val, read_only;
+ Sg_fd *sfp = filp->private_data;
Sg_device *sdp;
- Sg_fd *sfp;
Sg_request *srp;
unsigned long iflags;
- if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
+ if (!sfp)
+ return -ENXIO;
+ sdp = sfp->parentdp;
+ if (!sdp)
return -ENXIO;
SCSI_LOG_TIMEOUT(3, printk("sg_ioctl: %s, cmd=0x%x\n",
@@ -1093,14 +1108,17 @@ sg_unlocked_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
static long sg_compat_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
Sg_device *sdp;
- Sg_fd *sfp;
+ Sg_fd *sfp = filp->private_data;
struct scsi_device *sdev;
- if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
+ if (!sfp)
+ return -ENXIO;
+ sdp = sfp->parentdp;
+ if (!sdp)
return -ENXIO;
sdev = sdp->device;
- if (sdev->host->hostt->compat_ioctl) {
+ if (sdev->host->hostt->compat_ioctl) {
int ret;
ret = sdev->host->hostt->compat_ioctl(sdev, cmd_in, (void __user *)arg);
@@ -1116,15 +1134,18 @@ static unsigned int
sg_poll(struct file *filp, poll_table * wait)
{
unsigned int res = 0;
+ Sg_fd *sfp = filp->private_data;
Sg_device *sdp;
- Sg_fd *sfp;
Sg_request *srp;
int count = 0;
unsigned long iflags;
- if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp))
- || sfp->closed)
+ if (!sfp)
return POLLERR;
+ sdp = sfp->parentdp;
+ if ((!sdp) || sfp->closed)
+ return POLLERR;
+
poll_wait(filp, &sfp->read_wait, wait);
read_lock_irqsave(&sfp->rq_list_lock, iflags);
for (srp = sfp->headrp; srp; srp = srp->nextrp) {
@@ -1150,11 +1171,15 @@ sg_poll(struct file *filp, poll_table * wait)
static int
sg_fasync(int fd, struct file *filp, int mode)
{
+ Sg_fd *sfp = filp->private_data;
Sg_device *sdp;
- Sg_fd *sfp;
- if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
+ if (!sfp)
return -ENXIO;
+ sdp = sfp->parentdp;
+ if (!sdp)
+ return -ENXIO;
+
SCSI_LOG_TIMEOUT(3, printk("sg_fasync: %s, mode=%d\n",
sdp->disk->disk_name, mode));
@@ -1203,12 +1228,12 @@ static const struct vm_operations_struct sg_mmap_vm_ops = {
static int
sg_mmap(struct file *filp, struct vm_area_struct *vma)
{
- Sg_fd *sfp;
+ Sg_fd *sfp = filp->private_data;
unsigned long req_sz, len, sa;
Sg_scatter_hold *rsv_schp;
int k, length;
- if ((!filp) || (!vma) || (!(sfp = (Sg_fd *) filp->private_data)))
+ if ((!filp) || (!vma) || !(sfp))
return -ENXIO;
req_sz = vma->vm_end - vma->vm_start;
SCSI_LOG_TIMEOUT(3, printk("sg_mmap starting, vm_start=%p, len=%d\n",
--
1.7.0.4
On Thu, Jul 01, 2010 at 05:16:43PM +0400, Kulikov Vasiliy wrote:
> - Sg_fd *sfp;
> + Sg_fd *sfp = filp->private_data;
^^^^^^^^^^^^^^^^^^
Dereferenced here.
> unsigned long req_sz, len, sa;
> Sg_scatter_hold *rsv_schp;
> int k, length;
>
> - if ((!filp) || (!vma) || (!(sfp = (Sg_fd *) filp->private_data)))
> + if ((!filp) || (!vma) || !(sfp))
^^^^^
Checked here.
I obviously just spotted that during the review but another way would be
to use smatch to catch these. (http://smatch.sf.net)
$ /path/to/smatch_scripts/kchecker drivers/scsi/sg.c
CHK include/linux/version.h
CHK include/generated/utsrelease.h
CALL scripts/checksyscalls.sh
CHECK drivers/scsi/sg.c
drivers/scsi/sg.c +1236 sg_mmap(7) warn: variable dereferenced before check 'filp'
CC [M] drivers/scsi/sg.o
$
You could also get rid of the extra parenthesis.
+ if (!filp || !vma || !sfp)
> return -ENXIO;
> req_sz = vma->vm_end - vma->vm_start;
> SCSI_LOG_TIMEOUT(3, printk("sg_mmap starting, vm_start=%p, len=%d\n",
Btw. These are _way_ better than when you sent them the first time.
Thanks for doing resending them.
regards,
dan carpenter
On Thu, Jul 01, 2010 at 19:38 +0200, Dan Carpenter wrote:
> On Thu, Jul 01, 2010 at 05:16:43PM +0400, Kulikov Vasiliy wrote:
> > - Sg_fd *sfp;
> > + Sg_fd *sfp = filp->private_data;
> ^^^^^^^^^^^^^^^^^^
> Dereferenced here.
>
> > unsigned long req_sz, len, sa;
> > Sg_scatter_hold *rsv_schp;
> > int k, length;
> >
> > - if ((!filp) || (!vma) || (!(sfp = (Sg_fd *) filp->private_data)))
> > + if ((!filp) || (!vma) || !(sfp))
> ^^^^^
>
> Checked here.
Oops, I've loosed it... The problem is that this driver is not even compileable.
Patch v2 coming soon)
On Thu, Jul 01, 2010 at 22:27 +0400, Kulikov Vasiliy wrote:
> The problem is that this driver is not even compileable.
Forget about this.
staging/wlags49_h2/wl_pci.c is not compileable, not scsi/sg.c :)