2010-07-14 09:45:40

by Changli Gao

Subject: [PATCH v2] drivers: code cleanups

drivers: code cleanups

the first three goto clauses caused kfree(NULL), and the error code returned
by kset->uevent_ops->uevent() wasn't passed to the caller of show_uevent().
This patch also eliminates the unnecessary initializations of env and count.

Signed-off-by: Changli Gao <[email protected]>
----
drivers/base/core.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 38bbbd0..76cd4f0 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -274,9 +274,8 @@ static ssize_t show_uevent(struct device *dev, struct device_attribute *attr,
{
struct kobject *top_kobj;
struct kset *kset;
- struct kobj_uevent_env *env = NULL;
+ struct kobj_uevent_env *env;
int i;
- size_t count = 0;
int retval;

/* search the kset, the device belongs to */
@@ -284,16 +283,16 @@ static ssize_t show_uevent(struct device *dev, struct device_attribute *attr,
while (!top_kobj->kset && top_kobj->parent)
top_kobj = top_kobj->parent;
if (!top_kobj->kset)
- goto out;
+ return 0;

kset = top_kobj->kset;
if (!kset->uevent_ops || !kset->uevent_ops->uevent)
- goto out;
+ return 0;

/* respect filter */
if (kset->uevent_ops && kset->uevent_ops->filter)
if (!kset->uevent_ops->filter(kset, &dev->kobj))
- goto out;
+ return 0;

env = kzalloc(sizeof(struct kobj_uevent_env), GFP_KERNEL);
if (!env)
@@ -306,10 +305,10 @@ static ssize_t show_uevent(struct device *dev, struct device_attribute *attr,

/* copy keys to file */
for (i = 0; i < env->envp_idx; i++)
- count += sprintf(&buf[count], "%s\n", env->envp[i]);
+ retval += sprintf(&buf[retval], "%s\n", env->envp[i]);
out:
kfree(env);
- return count;
+ return retval;
}

static ssize_t store_uevent(struct device *dev, struct device_attribute *attr,

2010-07-14 10:03:35

by Oliver Neukum

Subject: Re: [PATCH v2] drivers: code cleanups

Am Mittwoch, 14. Juli 2010, 11:42:56 schrieb Changli Gao:
> drivers: code cleanups
>
> the first three goto clauses caused kfree(NULL), and the error code returned
> by kset->uevent_ops->uevent() wasn't passed to the caller of show_uevent().
> This patch also eliminates the unnecessary initializations of env and count.

This is not a good idea.
If you are indeed fixing a bug of not returning an error, make a patch that does that.
The other parts of your patch really are no improvement, because they make
things harder to maintain if additional cleanups or locking must ever be added.

Regards
Oliver