linux-kernel - Re: [PATCH v3 1/6] ACPI: dock: convert sysfs attributes to an attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091018071630.GC3935@core.coreip.homeip.net>
Date:	Sun, 18 Oct 2009 00:16:30 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Alex Chiang <achiang@...com>
Cc:	lenb@...nel.org, shaohua.li@...el.com,
	linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v3 1/6] ACPI: dock: convert sysfs attributes to an
	attribute_group

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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/