On Mon, Feb 13, 2017 at 11:14:14AM +0000, Ian Abbott wrote:
> On 11/02/17 10:37, Cheah Kok Cheong wrote:
> >+static int __init comedi_test_init(void)
> >+{
> >+    int ret;
> >+
> >+    ret = comedi_driver_register(&waveform_driver);
> >+    if (ret) {
> >+            pr_err("comedi_test: unable to register driver\n");
> >+            return ret;
> >+    }
> >+
> >+    if (!config_mode) {
> >+            ctcls = class_create(THIS_MODULE, CLASS_NAME);
> >+            if (IS_ERR(ctcls)) {
> >+                    pr_warn("comedi_test: unable to create class\n");
> >+                    return ret;
> >+            }
> >+
> >+            ctdev = device_create(ctcls, NULL, MKDEV(0, 0), NULL, DEV_NAME);
> >+            if (IS_ERR(ctdev)) {
> >+                    pr_warn("comedi_test: unable to create device\n");
> >+                    goto clean2;
> >+            }
> >+
> >+            ret = comedi_auto_config(ctdev, &waveform_driver, 0);
> >+            if (ret) {
> >+                    pr_warn("comedi_test: unable to auto-configure 
> >device\n");
> >+                    goto clean;
> >+            }
> >+    }
> >+
> >+    return 0;
> >+
> >+clean:
> >+    device_destroy(ctcls, MKDEV(0, 0));
> >+clean2:
> >+    class_destroy(ctcls);
> >+
> >+    return 0;
> >+}
> >+module_init(comedi_test_init);
> >+
> >+static void __exit comedi_test_exit(void)
> >+{
> >+    comedi_auto_unconfig(ctdev);
> >+
> >+    device_destroy(ctcls, MKDEV(0, 0));
> >+
> >+    class_destroy(ctcls);
> 
> If the driver init returned successfully, but failed to set-up the
> auto-configured device, the device and class will not exist at this point,
> so those three calls need to go in an 'if' statement.  Perhaps you could use
> 'if (ctcls) {', and set 'ctcls = NULL;' after calling
> 'class_destroy(ctcls);' in the init function.
> 
> Apart from that, it looks fine.
> 

Thanks for pointing out those two pointers have to be "NULL" if
auto-configuration fails.

Please review below snippet.
Sorry for the inconvenience caused.

[ Snip ]

static int __init comedi_test_init(void)
{
        int ret;

        ret = comedi_driver_register(&waveform_driver);
        if (ret) {
                pr_err("comedi_test: unable to register driver\n");
                return ret;
        }

        if (!config_mode) {
                ctcls = class_create(THIS_MODULE, CLASS_NAME);
                if (IS_ERR(ctcls)) {
                        pr_warn("comedi_test: unable to create class\n");
                        goto clean3;
                }

                ctdev = device_create(ctcls, NULL, MKDEV(0, 0), NULL, DEV_NAME);
                if (IS_ERR(ctdev)) {
                        pr_warn("comedi_test: unable to create device\n");
                        goto clean2;
                }

                ret = comedi_auto_config(ctdev, &waveform_driver, 0);
                if (ret) {
                        pr_warn("comedi_test: unable to auto-configure 
device\n");
                        goto clean;
                }
        }

        return 0;

clean:
        device_destroy(ctcls, MKDEV(0, 0));
clean2:
        class_destroy(ctcls);
        ctdev = NULL;
clean3:
        ctcls = NULL;

        return 0;
}
module_init(comedi_test_init);

static void __exit comedi_test_exit(void)
{
        if (ctdev)
                comedi_auto_unconfig(ctdev);

        if (ctcls) {
                device_destroy(ctcls, MKDEV(0, 0));
                class_destroy(ctcls);
        }

        comedi_driver_unregister(&waveform_driver);
}
module_exit(comedi_test_exit);

[ Snip ]

Thks.
Brgds,
CheahKC
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to