[PATCH 10/25] scsi/sg: remove casts from void*

[PATCH 10/25] scsi/sg: remove casts from void*

[Kulikov Vasiliy]

Remove unnesessary casts from void*.
Style fixes with assignments in if (...).

Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
 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

Re: [PATCH 10/25] scsi/sg: remove casts from void*

[Dan Carpenter]

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

Re: [PATCH 10/25] scsi/sg: remove casts from void*

[Kulikov Vasiliy]

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)

Re: [PATCH 10/25] scsi/sg: remove casts from void*

[Kulikov Vasiliy]

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 :)