[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <200812241157.33912.vapier@gentoo.org>
Date: Wed, 24 Dec 2008 11:57:31 -0500
From: Mike Frysinger <vapier@...too.org>
To: Adrian McMenamin <adrian@...golddream.dyndns.info>
Cc: LKML <linux-kernel@...r.kernel.org>,
"linux-sh" <linux-sh@...r.kernel.org>,
"linux-input" <linux-input@...r.kernel.org>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Paul Mundt <lethal@...ux-sh.org>, Andrew Morton <akpm@...l.org>
Subject: Re: [PATCH] sh: maple: add Maple controller as a joystick device
On Saturday 20 December 2008 13:26:54 Adrian McMenamin wrote:
> +static int probe_maple_controller(struct device *dev)
> +{
> ...
> + mdev = to_maple_dev(dev);
> + if (!mdev) {
> + error = EINVAL;
> + goto fail;
> + }
> +
> + mdrv = to_maple_driver(dev->driver);
> + if (!mdrv) {
> + error = EINVAL;
> + goto fail;
> + }
like Dmitry said, these NULL checks make no sense
> + pad = kzalloc(sizeof(struct dc_pad), GFP_KERNEL);
> + if (!pad) {
> + error = ENOMEM;
> + goto fail;
> + }
> + idev = input_allocate_device();
> + if (!idev){
> + error = ENOMEM;
> + goto fail_idev;
> + }
> ...
> + error = input_register_device(idev);
> + if (error)
> + goto fail_register;
> ...
> +fail_register:
> + maple_set_drvdata(mdev, NULL);
> + input_free_device(pad->dev);
> +fail_idev:
> + kfree(pad);
> +fail:
> + return -error;
> +}
this -error stuff is weird and i think wrong. for constants, you're adding
overhead, and for the input_register_device(), i think you're negating an
already negative value thus breaking it.
-mike
Download attachment "signature.asc " of type "application/pgp-signature" (836 bytes)
Powered by blists - more mailing lists