Return better error codes if drivers/char/raw.c module init fails

PATCH In review Rolf Eike Beer 3 participants 4 messages Aug 18, 2006 – Aug 20, 2006 Char and misc drivers

Share & tools

Rolf Eike Beer

author

2006-08-18 07:18 UTC #0 lore raw

PATCH 1/1 Return better error codes if drivers/char/raw.c module init fails

Currently this module just returns 1 if anything on module init fails. Store the error code of the different function calls and return their error on problems.

I'm not sure if this doesn't need even more cleanup, for example kobj_put() is called only in one error case.

Signed-off-by Rolf Eike Beer <[email protected]>

---
commit f6272846a16df0c7acb5c1701c0acdee0b472047
tree 441e29883ce4f449ca269b571e901ab617e0733c
parent 2cb2450818804edcbcb1486a4df0db06e5d49969
author Rolf Eike Beer <[email protected]> Fri, 18 Aug 2006 09:13:03 +0200 committer Rolf Eike Beer <[email protected]> Fri, 18 Aug 2006 09:13:03 +0200

 drivers/char/raw.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

drivers/char/raw.c +12 -8

index 579868a..5938e6b 100644
@@ -288,31 +288,35 @@ static struct cdev raw_cdev = {
 static int __init raw_init(void)
 {
 	dev_t dev = MKDEV(RAW_MAJOR, 0);
+	int ret;
 
-	if (register_chrdev_region(dev, MAX_RAW_MINORS, "raw"))
+	ret = register_chrdev_region(dev, MAX_RAW_MINORS, "raw");
+	if (ret)
 		goto error;
 
 	cdev_init(&raw_cdev, &raw_fops);
-	if (cdev_add(&raw_cdev, dev, MAX_RAW_MINORS)) {
+	ret = cdev_add(&raw_cdev, dev, MAX_RAW_MINORS); 
+	if (ret) {
+		printk(KERN_ERR "error register raw device\n");
 		kobject_put(&raw_cdev.kobj);
-		unregister_chrdev_region(dev, MAX_RAW_MINORS);
-		goto error;
+		goto error_region;
 	}
 
 	raw_class = class_create(THIS_MODULE, "raw");
 	if (IS_ERR(raw_class)) {
 		printk(KERN_ERR "Error creating raw class.\n");
 		cdev_del(&raw_cdev);
-		unregister_chrdev_region(dev, MAX_RAW_MINORS);
-		goto error;
+		ret = PTR_ERR(raw_class);
+		goto error_region;
 	}
 	class_device_create(raw_class, NULL, MKDEV(RAW_MAJOR, 0), NULL, "rawctl");
 
 	return 0;
 
+error_region:
+	unregister_chrdev_region(dev, MAX_RAW_MINORS);
 error:
-	printk(KERN_ERR "error register raw device\n");
-	return 1;
+	return ret;
 }
 
 static void __exit raw_exit(void)

Andrew Morton

2006-08-18 23:27 UTC #1 lore raw

Quoted 7 lines · On Fri, 18 Aug 2006 09:18:30 +0200 Rolf Eike Beer <[email protected]> wrote:

Currently this module just returns 1 if anything on module init fails. Store the error code of the different function calls and return their error on problems.
I'm not sure if this doesn't need even more cleanup, for example kobj_put() is called only in one error case.

You seem to be using kmail in funky-confuse-sylpheed mode. Inlined patches in plain-text emails are preferred, please.

Quoted 37 lines

drivers/char/raw.c +9 -6

index 579868a..5938e6b 100644
@@ -288,31 +288,35 @@ static struct cdev raw_cdev = {
 static int __init raw_init(void)
 {
 	dev_t dev = MKDEV(RAW_MAJOR, 0);
+	int ret;
 
-	if (register_chrdev_region(dev, MAX_RAW_MINORS, "raw"))
+	ret = register_chrdev_region(dev, MAX_RAW_MINORS, "raw");
+	if (ret)
 		goto error;
 
 	cdev_init(&raw_cdev, &raw_fops);
-	if (cdev_add(&raw_cdev, dev, MAX_RAW_MINORS)) {
+	ret = cdev_add(&raw_cdev, dev, MAX_RAW_MINORS); 
+	if (ret) {
+		printk(KERN_ERR "error register raw device\n");
 		kobject_put(&raw_cdev.kobj);
-		unregister_chrdev_region(dev, MAX_RAW_MINORS);
-		goto error;
+		goto error_region;
 	}
 
 	raw_class = class_create(THIS_MODULE, "raw");
 	if (IS_ERR(raw_class)) {
 		printk(KERN_ERR "Error creating raw class.\n");
 		cdev_del(&raw_cdev);
-		unregister_chrdev_region(dev, MAX_RAW_MINORS);
-		goto error;
+		ret = PTR_ERR(raw_class);
+		goto error_region;
 	}
 	class_device_create(raw_class, NULL, MKDEV(RAW_MAJOR, 0), NULL, "rawctl");

drivers/char/raw.c static struct cdev raw_cdev = { · line 320

Quoted 10 lines

drivers/char/raw.c +3 -2

 
 	return 0;
 
+error_region:
+	unregister_chrdev_region(dev, MAX_RAW_MINORS);
 error:
-	printk(KERN_ERR "error register raw device\n");
-	return 1;
+	return ret;
 }

No, it's not obvious what that stray kobject_put() is doing in there.

<hunt, hunt>

http://kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=commitdiff;h=b8ff72d28c349bdb7ff5246e83aba384f45d8078

Randy.Dunlap

2006-08-18 23:34 UTC #2 lore raw

Quoted 7 lines · On Fri, 18 Aug 2006 16:27:43 -0700 Andrew Morton wrote:

Quoted 4 lines · On Fri, 18 Aug 2006 09:18:30 +0200 Rolf Eike Beer <[email protected]> wrote:
Currently this module just returns 1 if anything on module init fails. Store the error code of the different function calls and return their error on problems.

Quoted 6 lines

Quoted 3 lines
I'm not sure if this doesn't need even more cleanup, for example kobj_put() is called only in one error case.
You seem to be using kmail in funky-confuse-sylpheed mode. Inlined patches in plain-text emails are preferred, please.

Something about the SLIM docum. patch did that too, although it was not an attachment.
Sylpheed had trouble with the line endings....
Saving the patch to a file works well, but replying to it looks like garbage.

---
~Randy

Rolf Eike Beer

author

2006-08-20 15:22 UTC #3 lore raw

In reply to Andrew Morton

Quoted 7 lines · Andrew Morton

On Fri, 18 Aug 2006 09:18:30 +0200
Quoted 4 lines · Rolf Eike Beer
Currently this module just returns 1 if anything on module init fails. Store the error code of the different function calls and return their error on problems.

Quoted 5 lines

Quoted 2 lines
I'm not sure if this doesn't need even more cleanup, for example kobj_put() is called only in one error case.
You seem to be using kmail in funky-confuse-sylpheed mode. Inlined patches in plain-text emails are preferred, please.

Sorry, I left the "sign mail" activated by accident. gpg-agent had the password still cached, otherwise I would have seen that.

Eike