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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/