Hi Alex, On Fri, Oct 16, 2009 at 03:14:59PM -0600, Alex Chiang wrote: > As suggested by Dmitry Torokhov, convert the individual sysfs > attributes into an attribute group. > > This change eliminates quite a bit of copy/paste code in the > error handling paths. > Looks much better, one more suggestion though: > +err_unregister: > + printk(KERN_ERR "%s encountered error %d\n", __func__, ret); If you want to print error this it should probably go down, right before "return ret". > + sysfs_remove_group(&dock_device->dev.kobj, &dock_attribute_group); It begs another label right here. There are cases when yo0u already registered the platform device but haven't added the sysfs group, right? > platform_device_unregister(dock_device); > +out: > kfree(dock_station); > dock_station = NULL; > return ret; > @@ -1076,11 +1046,7 @@ static int dock_remove(struct dock_station *dock_station) > kfree(dd); > > /* cleanup sysfs */ > - device_remove_file(&dock_device->dev, &dev_attr_type); > - device_remove_file(&dock_device->dev, &dev_attr_docked); > - device_remove_file(&dock_device->dev, &dev_attr_undock); > - device_remove_file(&dock_device->dev, &dev_attr_uid); > - device_remove_file(&dock_device->dev, &dev_attr_flags); > + sysfs_remove_group(&dock_device->dev.kobj, &dock_attribute_group); > platform_device_unregister(dock_device); > > /* free dock station memory */ > -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html