[Cbe-oss-dev] [patch 10/13] powerpc: sysfs fix compiler warning

[Michael Ellerman]

On Thu, 2007-07-19 at 00:32 +0200, Arnd Bergmann wrote:
> On Wednesday 18 July 2007, Andrew Morton wrote:
> > >       for_each_possible_cpu(cpu) {
> > >               sysdev = get_cpu_sysdev(cpu);
> > > -             sysfs_create_group(&sysdev->kobj, attrs);
> > > +             error = sysfs_create_group(&sysdev->kobj, attrs);
> > > +
> > > +             if (error) {
> > > +                     for_each_possible_cpu(cpu) {
> > > +                             sysdev = get_cpu_sysdev(cpu);
> > > +                             sysfs_remove_group(&sysdev->kobj, attrs);
> > > +                     }
> > > +                     break;
> > > +             }
> > >       }
> > >  
> > >       mutex_unlock(&cpu_mutex);
> > > -     return 0;
> > > +     return error;
> > >  }
> > >  EXPORT_SYMBOL_GPL(cpu_add_sysdev_attr_group);
> > 
> > This will attempt to remove groups which were never added.  I don't know
> > what will happen then.

It might BUG if the directory lookup fails, otherwise it will happily do
nothing AFAICT.

> I looked at the code and it doesn't look too good. Let's drop this patch for
> now. Christian, please submit a new patch when you've found a solution.


Given that we __must_check the create, it'd be nice if the sysfs API
gave us a bit of help, how about:


diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 52eed2a..cba7a2c 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -65,15 +65,16 @@ int sysfs_create_group(struct kobject * kobj,
 	return error;
 }
 
-void sysfs_remove_group(struct kobject * kobj, 
-			const struct attribute_group * grp)
+static int __sysfs_remove_group(struct kobject * kobj, 
+			 const struct attribute_group * grp)
 {
 	struct dentry * dir;
 
 	if (grp->name) {
 		dir = lookup_one_len_kern(grp->name, kobj->dentry,
 				strlen(grp->name));
-		BUG_ON(IS_ERR(dir));
+		if (IS_ERR(dir))
+			return PTR_ERR(dir);
 	}
 	else
 		dir = dget(kobj->dentry);
@@ -83,8 +84,22 @@ void sysfs_remove_group(struct kobject * kobj,
 		sysfs_remove_subdir(dir);
 	/* release the ref. taken in this routine */
 	dput(dir);
+
+	return 0;
+}
+
+void sysfs_remove_group(struct kobject * kobj, 
+			const struct attribute_group * grp)
+{
+	BUG_ON(__sysfs_remove_group(kobj, grp));
 }
+EXPORT_SYMBOL_GPL(sysfs_remove_group);
 
+void sysfs_remove_group_please(struct kobject * kobj, 
+			const struct attribute_group * grp)
+{
+	__sysfs_remove_group(kobj, grp);
+}
+EXPORT_SYMBOL_GPL(sysfs_remove_group_please);
 
 EXPORT_SYMBOL_GPL(sysfs_create_group);
-EXPORT_SYMBOL_GPL(sysfs_remove_group);