linux-kernel - Re: [Patch 3/4] ST LIS3L02DQ accelerometer

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080723180726.6fc0ef6e@lxorguk.ukuu.org.uk>
Date:	Wed, 23 Jul 2008 18:07:26 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Jonathan Cameron <Jonathan.Cameron@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	spi-devel-general@...ts.sourceforge.net,
	LM Sensors <lm-sensors@...sensors.org>,
	Jean Delvare <khali@...ux-fr.org>,
	Dmitry Torokhov <dtor@...l.ru>,
	"Hans J. Koch" <hjk@...utronix.de>, hmh@....eng.br,
	David Brownell <david-b@...bell.net>, mgross@...ux.intel.com,
	Ben Nizette <bn@...sdigital.com>,
	Anton Vorontsov <avorontsov@...mvista.com>
Subject: Re: [Patch 3/4] ST LIS3L02DQ accelerometer

> +	xfer.tx_buf = kmalloc(4, GFP_KERNEL);
> +	if (xfer.tx_buf == NULL) {
> +		ret = -ENOMEM;
> +		goto error_ret;
> +	}
> +
> +	xfer.rx_buf = kmalloc(4, GFP_KERNEL);
> +	if (xfer.rx_buf == NULL) {
> +		ret = -ENOMEM;
> +		goto error_free_tx;
> +	}

Do these really need to be two kmallocs

> +	if (ret) {
> +		dev_err(&st->us->dev, "problem with get x offset");
> +		goto error_free_rx;
> +	}
> +	*val = ((unsigned char *)(xfer.rx_buf))[0];
> +	kfree(xfer.rx_buf);
> +	kfree(xfer.tx_buf);
> +	return ret;
> +error_free_rx:
> +	kfree(xfer.rx_buf);
> +error_free_tx:
> +	kfree(xfer.tx_buf);
> +error_ret:
> +	return ret;

That lot makes no sense. You could just drop through..

> +{
> +	uint8_t val;
> +	int8_t ret;

Kernel types (u8, s8 etc)

>
> +	local_rx_buf = kmalloc(4, GFP_KERNEL);
> +	local_tx_buf = kmalloc(4, GFP_KERNEL);
> +
> +	local_tx_buf[1] = LIS3L02DQ_READ_REG(reg_address);

OOPS on out of memory case


> +	struct spi_transfer xfer = {
> +		.tx_buf = NULL,
> +		.rx_buf = NULL,
> +		.bits_per_word = 16,
> +		.len = 2,
> +	};
> +	int ret;
> +
> +	local_tx_buf = kmalloc(4, GFP_KERNEL);
> +	if (local_tx_buf == NULL) {
> +		ret = -ENOMEM;
> +		goto error_ret;
> +	}
> +	xfer.tx_buf = local_tx_buf;

You seem to have a lot of almost identical routines here. It looks like
with a few helpers this driver could be vastly shorter.
--
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/