Messages in this thread | | | From | Dmitry Torokhov <> | Subject | Re: [PATCH] sh: maple: add support for Maple controller as a joystick | Date | Sat, 20 Dec 2008 12:19:12 -0800 |
| |
On Saturday 20 December 2008 08:07:27 Adrian McMenamin wrote: > On Sat, 2008-12-20 at 15:16 +0000, Matt Fleming wrote: > > On Fri, Dec 19, 2008 at 11:15:42PM +0000, Adrian McMenamin wrote: > > > Add support for the SEGA Dreamcast Maple controller as a joystick > > > > [snip] > > > > > + struct dc_pad *pad = maple_get_drvdata(mapledev); > > > + struct input_dev *dev = pad->dev; > > > + unsigned char *res = mq->recvbuf; > > > + > > > + buttons = ~cpu_to_le16(*(unsigned short *)(res + 8)); > > > + > > > > I may be wrong but shouldn't this use the I/O accessor functions? > > It's not iomemory, so I don't think so. The maple bus writes to a buffer > in the physical RAM. >
Hmm, but why we are going _from_ cpu order to le16??? I'd expect we wanted to do conversion the other way around. I pulled it out of 'next' for now.
> > > + > > > + pad = kzalloc(sizeof(struct dc_pad), GFP_KERNEL); > > > + idev = input_allocate_device(); > > > + if (!pad || !idev){ > > > + error = ENOMEM; > > > + goto fail; > > > + } > > > > [snip] > > > > > +fail: > > > + input_free_device(pad->dev); > > > > Possible NULL pointer dereference? > > No, because input_free_device is: > > void input_free_device(struct input_dev *dev) > { > if (dev) > input_put_device(dev); > } > EXPORT_SYMBOL(input_free_device);
But we need to be freeing idev, not pad->dev because pad might be NULL.
-- Dmitry
| |