linux-kernel - Re: [PATCH 09/12] hwmon: vexpress: Use devm helper for hwmon device registration

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: <201402112157.46774.arnd@arndb.de>
Date:	Tue, 11 Feb 2014 21:57:46 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Pawel Moll <pawel.moll@....com>
Cc:	arm@...nel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, Jean Delvare <jdelvare@...e.de>,
	Guenter Roeck <linux@...ck-us.net>, lm-sensors@...sensors.org
Subject: Re: [PATCH 09/12] hwmon: vexpress: Use devm helper for hwmon device registration

On Tuesday 11 February 2014, Pawel Moll wrote:
> -#define VEXPRESS_HWMON_ATTRS(_name, _label_attr, _input_attr)  \
> -struct attribute *vexpress_hwmon_attrs_##_name[] = {           \
> -       &dev_attr_name.attr,                                    \
> -       &dev_attr_##_label_attr.attr,                           \
> -       &sensor_dev_attr_##_input_attr.dev_attr.attr,           \
> -       NULL                                                    \
> -}
> +#define VEXPRESS_HWMON_ATTR_GROUPS(_name, _label_attr, _input_attr)    \
> +static struct attribute *vexpress_hwmon_##_name##_attrs[] = {          \
> +       &dev_attr_##_label_attr.attr,                                   \
> +       &sensor_dev_attr_##_input_attr.dev_attr.attr,                   \
> +       NULL                                                            \
> +};                                                                     \
> +ATTRIBUTE_GROUPS(vexpress_hwmon_##_name)
>  
>  #if !defined(CONFIG_REGULATOR_VEXPRESS)
>  static DEVICE_ATTR(in1_label, S_IRUGO, vexpress_hwmon_label_show, NULL);
>  static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, vexpress_hwmon_u32_show,
>                 NULL, 1000);
> -static VEXPRESS_HWMON_ATTRS(volt, in1_label, in1_input);
> -static struct attribute_group vexpress_hwmon_group_volt = {
> -       .attrs = vexpress_hwmon_attrs_volt,
> -};
> +VEXPRESS_HWMON_ATTR_GROUPS(volt, in1_label, in1_input);

Maybe it would be a good time to get rid of the obfuscation here and just
open-code the attribute groups rather than defining another macro.

I understand you want to save a few source lines, but it really helps
readability if you can search for the attribute groups when they
are referenced later. Just because the common driver model code uses
macros with string concatenation doesn't mean it's a good idea to
do the same in a device driver.

	Arnd
--
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/