On Wed, Aug 14, 2019 at 06:57:55PM +0200, Jonas Gorski wrote:

On Wed, 14 Aug 2019 at 16:37, Thomas Bogendoerfer [off-list ref] wrote:

quoted

On Wed, 14 Aug 2019 15:20:14 +0200
Jonas Gorski [off-list ref] wrote:

quoted

quoted

+       d = devm_kzalloc(&pdev->dev, sizeof(*d), GFP_KERNEL);

&pdev->dev => dev

will change.

quoted

quoted

+       if (!d)
+               return -ENOMEM;
+
+       sk = kzalloc(sizeof(*sk), GFP_KERNEL);

any reason not to devm_kzalloc this as well? Then you won't need to
manually free it in the error cases.

it has different life time than the device, so it may not allocated
via devm_kzalloc

quoted

quoted

+static int ioc3kbd_remove(struct platform_device *pdev)
+{
+       struct ioc3kbd_data *d = platform_get_drvdata(pdev);
+
+       devm_free_irq(&pdev->dev, d->irq, d);
+       serio_unregister_port(d->kbd);
+       serio_unregister_port(d->aux);
+       return 0;
+}

and on that topic, won't you need to kfree d->kbd and d->aux here?

that's done in serio_release_port() by the serio core.

i see. But in that case, don't the kfree's after the
serio_unregister_port's in the error path of the .probe function cause
a double free?

Yes they do, we need to drop kfree()s from there. Nicely spotted.

Thanks.

-- 
Dmitry