2006-08-18 07:18:15

by Rolf Eike Beer

[permalink] [raw]
Subject: [PATCH][CHAR] 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(-)

diff --git a/drivers/char/raw.c b/drivers/char/raw.c
index 579868a..5938e6b 100644
--- a/drivers/char/raw.c
+++ b/drivers/char/raw.c
@@ -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)


Attachments:
(No filename) (2.00 kB)
(No filename) (189.00 B)
Download all attachments

2006-08-18 23:27:51

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH][CHAR] Return better error codes if drivers/char/raw.c module init fails

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.

>
> diff --git a/drivers/char/raw.c b/drivers/char/raw.c
> index 579868a..5938e6b 100644
> --- a/drivers/char/raw.c
> +++ b/drivers/char/raw.c
> @@ -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;
> }

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


2006-08-18 23:34:55

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH][CHAR] Return better error codes if drivers/char/raw.c module init fails

On Fri, 18 Aug 2006 16:27:43 -0700 Andrew Morton wrote:

> 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.

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

2006-08-20 15:22:24

by Rolf Eike Beer

[permalink] [raw]
Subject: Re: [PATCH][CHAR] Return better error codes if drivers/char/raw.c module init fails

Andrew Morton wrote:
> 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.

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

Eike


Attachments:
(No filename) (664.00 B)
(No filename) (189.00 B)
Download all attachments