[PATCH] scsi/osst.c: remove unncessary casting of kmalloc.

[PATCH] scsi/osst.c: remove unncessary casting of kmalloc.

[Thiago Farina]

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 drivers/scsi/osst.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index acb8358..72d1e79 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -5842,9 +5842,8 @@ static int osst_probe(struct device *dev)
 	/* if this is the first attach, build the infrastructure */
 	write_lock(&os_scsi_tapes_lock);
 	if (os_scsi_tapes == NULL) {
-		os_scsi_tapes =
-			(struct osst_tape **)kmalloc(osst_max_dev * sizeof(struct osst_tape *),
-				   GFP_ATOMIC);
+		os_scsi_tapes = kmalloc(osst_max_dev * sizeof(struct osst_tape *),
+					GFP_ATOMIC);
 		if (os_scsi_tapes == NULL) {
 			write_unlock(&os_scsi_tapes_lock);
 			printk(KERN_ERR "osst :E: Unable to allocate array for OnStream SCSI tapes.\n");
@@ -5852,7 +5851,7 @@ static int osst_probe(struct device *dev)
 		}
 		for (i=0; i < osst_max_dev; ++i) os_scsi_tapes[i] = NULL;
 	}
-	
+
 	if (osst_nr_dev >= osst_max_dev) {
 		write_unlock(&os_scsi_tapes_lock);
 		printk(KERN_ERR "osst :E: Too many tape devices (max. %d).\n", osst_max_dev);
@@ -5917,7 +5916,7 @@ static int osst_probe(struct device *dev)
 	tpnt->os_fw_rev = osst_parse_firmware_rev (SDp->rev);
 	tpnt->omit_blklims = 1;
 
-	tpnt->poll = (strncmp(SDp->model, "DI-", 3) == 0) || 
+	tpnt->poll = (strncmp(SDp->model, "DI-", 3) == 0) ||
 		     (strncmp(SDp->model, "FW-", 3) == 0) || OSST_FW_NEED_POLL(tpnt->os_fw_rev,SDp);
 	tpnt->frame_in_buffer = 0;
 	tpnt->header_ok = 0;
-- 
1.6.6.103.g699d2

Re: [PATCH] scsi/osst.c: remove unncessary casting of kmalloc.

[Stefan Richter]

Thiago Farina wrote:
> --- a/drivers/scsi/osst.c
> +++ b/drivers/scsi/osst.c
> @@ -5842,9 +5842,8 @@ static int osst_probe(struct device *dev)
>  	/* if this is the first attach, build the infrastructure */
>  	write_lock(&os_scsi_tapes_lock);
>  	if (os_scsi_tapes == NULL) {
> -		os_scsi_tapes =
> -			(struct osst_tape **)kmalloc(osst_max_dev * sizeof(struct osst_tape *),
> -				   GFP_ATOMIC);
> +		os_scsi_tapes = kmalloc(osst_max_dev * sizeof(struct osst_tape *),
> +					GFP_ATOMIC);
>  		if (os_scsi_tapes == NULL) {
>  			write_unlock(&os_scsi_tapes_lock);
>  			printk(KERN_ERR "osst :E: Unable to allocate array for OnStream SCSI tapes.\n");

Since you update the style of this kmalloc usage, you could at the same
time change the sizeof expression to sizeof(* os_scsi_tapes).

There is a lot more that could be renovated around os_scsi_tapes, and
osst in general (GFP_ATOMIC allocations in a device probe?  Fixed
maximum number of devices?  BKL usage?), but whether it'd be worth the
effort I don't know.
-- 
Stefan Richter
-=====-==-=- ---= =----
http://arcgraph.de/sr/

Re: [PATCH] scsi/osst.c: remove unncessary casting of kmalloc.

[Stefan Richter]

Stefan Richter wrote:
> Thiago Farina wrote:
>> -		os_scsi_tapes =
>> -			(struct osst_tape **)kmalloc(osst_max_dev * sizeof(struct osst_tape *),
>> -				   GFP_ATOMIC);
>> +		os_scsi_tapes = kmalloc(osst_max_dev * sizeof(struct osst_tape *),
>> +					GFP_ATOMIC);

> Since you update the style of this kmalloc usage, you could at the same
> time change the sizeof expression to sizeof(* os_scsi_tapes).

PS: osst_max_dev * sizeof(* os_scsi_tapes) of course, if it wasn't obvious.
-- 
Stefan Richter
-=====-==-=- ---= =----
http://arcgraph.de/sr/

Re: [PATCH] scsi/osst.c: remove unncessary casting of kmalloc.

[James Bottomley]

On Sat, 2010-01-16 at 16:35 +0100, Stefan Richter wrote:
> Thiago Farina wrote:
> > --- a/drivers/scsi/osst.c
> > +++ b/drivers/scsi/osst.c
> > @@ -5842,9 +5842,8 @@ static int osst_probe(struct device *dev)
> >  	/* if this is the first attach, build the infrastructure */
> >  	write_lock(&os_scsi_tapes_lock);
> >  	if (os_scsi_tapes == NULL) {
> > -		os_scsi_tapes =
> > -			(struct osst_tape **)kmalloc(osst_max_dev * sizeof(struct osst_tape *),
> > -				   GFP_ATOMIC);
> > +		os_scsi_tapes = kmalloc(osst_max_dev * sizeof(struct osst_tape *),
> > +					GFP_ATOMIC);
> >  		if (os_scsi_tapes == NULL) {
> >  			write_unlock(&os_scsi_tapes_lock);
> >  			printk(KERN_ERR "osst :E: Unable to allocate array for OnStream SCSI tapes.\n");
> 
> Since you update the style of this kmalloc usage, you could at the same
> time change the sizeof expression to sizeof(* os_scsi_tapes).

Actually, no, please don't do any of this without explicitly checking
with the maintainer (Willem).  We allow fairly loose rein in style for
individual drivers so random minor style changes aren't accepted without
explicit maintainer ack.  For unmaintained files, I also tend to err on
the side of trying not to alter the file unless necessary, so again it
has to be more than just a minor style problem.

For major and reasonable stye changes in unmaintained drivers, an md5sum
of the output code is necessary (verifying it to be the same as the
previous code) unless you can do some testing.

> There is a lot more that could be renovated around os_scsi_tapes, and
> osst in general (GFP_ATOMIC allocations in a device probe?  Fixed
> maximum number of devices?  BKL usage?), but whether it'd be worth the
> effort I don't know.

So this would be a more worthwhile update.  *however* changes along
these lines would actually need to be tested, so someone needs to verify
they work somehow.

James