linux-kernel - Re: [PATCH RFC] sh: maple: Add support for SEGA Dreamcast VMU and clean up maple bus driver (1/3)

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: <200901281942.22332.vapier@gentoo.org>
Date:	Wed, 28 Jan 2009 19:42:20 -0500
From:	Mike Frysinger <vapier@...too.org>
To:	Adrian McMenamin <adrian@...golddream.dyndns.info>
Cc:	Paul Mundt <lethal@...ux-sh.org>, Greg KH <greg@...ah.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	dwmw2 <dwmw2@...radead.org>,
	"linux-sh" <linux-sh@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-input" <linux-input@...r.kernel.org>,
	"linux-mtd" <linux-mtd@...r.kernel.org>
Subject: Re: [PATCH RFC] sh: maple: Add support for SEGA Dreamcast VMU and clean up maple bus driver (1/3)

On Wednesday 28 January 2009 18:57:49 Adrian McMenamin wrote:
> --- a/drivers/sh/maple/maple.c
> +++ b/drivers/sh/maple/maple.c
> -int maple_add_packet_sleeps(struct maple_device *mdev, u32 function,
> -	u32 command, size_t length, void *data)
> -{
> -	int locking, ret = 0;
> -	void *sendbuf = NULL;
> -
> -	locking = mutex_lock_interruptible(&mdev->mq->mutex);
> -	if (locking) {
> -		ret = -EIO;
> -		goto out;
> -	}
> +	int ret = 0;
> +	void *sendbuf;
>
> +	sendbuf = kzalloc(length * 4, GFP_KERNEL);
>  	if (length) {
> -		sendbuf = kmalloc(length * 4, GFP_KERNEL);

that looks wonky ... you do the alloc first and then check the len ?

>  failed_nomem:
> +	printk(KERN_INFO "maple: could not allocate memory\n");

should be a KERN_NOTICE or worse ... KERN_INFO doesnt seem appropriate

> +	if (device_register(&mdev->dev)) {
>  		printk(KERN_INFO
> -		"Maple bus: Attempt to register device"
> -		" (%x, %x) failed.\n",
> +			"Maple bus: Attempt to register device"
> +			" (%x, %x) failed.\n",

same here ... KERN_INFO shouldnt be used in error handlers

> +static void maple_response_fileerr(struct maple_device *mdev, void
> *recvbuf) +{
> +	if (mdev->fileerrhandler) {
> +		mdev->fileerrhandler(mdev, recvbuf);
> +		return;
> +	} else
> +		printk(KERN_INFO "maple: device (%d, %d) reports file error %d\n",
> +			mdev->port, mdev->unit, ((int *)recvbuf)[1]);
> +}

and here ...

>  			case MAPLE_RESPONSE_AGAIN:
>  			case MAPLE_RESPONSE_BADCMD:
>  			case MAPLE_RESPONSE_BADFUNC:
> -				printk(KERN_DEBUG
> +				printk(KERN_INFO
>  				       "Maple non-fatal error 0x%X\n",
>  				       code);

and here ...

>  /* Maple Bus command and response codes */
>  enum maple_code {
> +	MAPLE_RESPONSE_FILEERR =	-5,
> +	MAPLE_RESPONSE_AGAIN =		-4,	/* retransmit */

do you really need to set every value ?  i would only bother when there's a 
discontinuity ...
-mike

Download attachment "signature.asc " of type "application/pgp-signature" (836 bytes)