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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sat, 16 Jan 2010 09:46:08 -0600
From:	James Bottomley <James.Bottomley@...e.de>
To:	Stefan Richter <stefanr@...6.in-berlin.de>
Cc:	Thiago Farina <tfransosi@...il.com>, linux-kernel@...r.kernel.org,
	Willem Riede <osst@...de.org>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	osst-users@...ts.sourceforge.net, linux-scsi@...r.kernel.org
Subject: Re: [PATCH] scsi/osst.c: remove unncessary casting of kmalloc.

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/