linux-kernel - Re: [PATCH 2/2] HID: roccat: using new sysfs_create_bin

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTik4qPyBtKgJwHVnhkTASNORoKrWms6gk8NirYeU@mail.gmail.com>
Date:	Fri, 12 Nov 2010 16:21:45 -0800
From:	Eric Biederman <ebiederm@...stanetworks.com>
To:	Stefan Achatz <stefan_achatz@....de>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>,
	"Serge E. Hallyn" <serue@...ibm.com>, Tejun Heo <tj@...nel.org>,
	Jiri Kosina <jkosina@...e.cz>, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Benjamin Thery <benjamin.thery@...l.net>
Subject: Re: [PATCH 2/2] HID: roccat: using new sysfs_create_bin_group() in
 kone driver

On Fri, Nov 12, 2010 at 10:18 AM, Stefan Achatz <stefan_achatz@....de> wrote:
> hid-roccat-kone now uses new group functions for creating binary
> sysfs attributes.

Looking at this, I have a problem with the way this works.
You are still doing this the hard and racy way.

sysfs attributes that are only added when we initialize the hardware and
are only removed when we remove the driver should use the device layer
functions to create their attributes.

This achieves two things.  The code is easier to write because there
is less of it.
The notification to user space happens after the attributes appear so
that you don't
have strange hotplug races.

If there a chance you can look at implementing this in the simpler
race free way?

Eric


>
> Signed-off-by: Stefan Achatz <erazor_de@...rs.sourceforge.net>
> ---
>  drivers/hid/hid-roccat-kone.c |   53 ++++++++++++----------------------------
>  1 files changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
> index f776957..43f1693 100644
> --- a/drivers/hid/hid-roccat-kone.c
> +++ b/drivers/hid/hid-roccat-kone.c
> @@ -710,6 +710,20 @@ static struct bin_attribute kone_profile5_attr = {
>        .write = kone_sysfs_write_profile5
>  };
>
> +static struct attribute *kone_bin_attributes[] = {
> +       &kone_settings_attr.attr,
> +       &kone_profile1_attr.attr,
> +       &kone_profile2_attr.attr,
> +       &kone_profile3_attr.attr,
> +       &kone_profile4_attr.attr,
> +       &kone_profile5_attr.attr,
> +       NULL
> +};
> +
> +static struct attribute_group kone_bin_attribute_group = {
> +       .attrs = kone_bin_attributes
> +};
> +
>  static int kone_create_sysfs_attributes(struct usb_interface *intf)
>  {
>        int retval;
> @@ -718,42 +732,12 @@ static int kone_create_sysfs_attributes(struct usb_interface *intf)
>        if (retval)
>                goto exit_1;
>
> -       retval = sysfs_create_bin_file(&intf->dev.kobj, &kone_settings_attr);
> +       retval = sysfs_create_bin_group(&intf->dev.kobj, &kone_bin_attribute_group);
>        if (retval)
>                goto exit_2;
>
> -       retval = sysfs_create_bin_file(&intf->dev.kobj, &kone_profile1_attr);
> -       if (retval)
> -               goto exit_3;
> -
> -       retval = sysfs_create_bin_file(&intf->dev.kobj, &kone_profile2_attr);
> -       if (retval)
> -               goto exit_4;
> -
> -       retval = sysfs_create_bin_file(&intf->dev.kobj, &kone_profile3_attr);
> -       if (retval)
> -               goto exit_5;
> -
> -       retval = sysfs_create_bin_file(&intf->dev.kobj, &kone_profile4_attr);
> -       if (retval)
> -               goto exit_6;
> -
> -       retval = sysfs_create_bin_file(&intf->dev.kobj, &kone_profile5_attr);
> -       if (retval)
> -               goto exit_7;
> -
>        return 0;
>
> -exit_7:
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile4_attr);
> -exit_6:
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile3_attr);
> -exit_5:
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile2_attr);
> -exit_4:
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile1_attr);
> -exit_3:
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_settings_attr);
>  exit_2:
>        sysfs_remove_group(&intf->dev.kobj, &kone_attribute_group);
>  exit_1:
> @@ -762,12 +746,7 @@ exit_1:
>
>  static void kone_remove_sysfs_attributes(struct usb_interface *intf)
>  {
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile5_attr);
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile4_attr);
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile3_attr);
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile2_attr);
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_profile1_attr);
> -       sysfs_remove_bin_file(&intf->dev.kobj, &kone_settings_attr);
> +       sysfs_remove_group(&intf->dev.kobj, &kone_bin_attribute_group);
>        sysfs_remove_group(&intf->dev.kobj, &kone_attribute_group);
>  }
>
> --
> 1.7.2.3
>
>
>
>
--
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/