linux-kernel - Re: [PATCH 1/3] hwmon: (applesmc) Allow format checking

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] [day] [month] [year] [list]

Message-ID: <54DD87FF.7070803@roeck-us.net>
Date:	Thu, 12 Feb 2015 21:13:35 -0800
From:	Guenter Roeck <linux@...ck-us.net>
To:	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Henrik Rydberg <rydberg@...math.org>,
	Jean Delvare <jdelvare@...e.de>
CC:	lm-sensors@...sensors.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] hwmon: (applesmc) Allow format checking

On 02/12/2015 06:15 AM, Rasmus Villemoes wrote:
> Currently gcc and other tools can't check the format strings. It's
> easy to fix by letting fan_speed_fmt simply hold what is different
> between the strings (and renaming it appropriately). While at it, we
> can also eliminate some wasted space and an extra level of indirection
> by making it an array of char[4] instead of char*.
>
> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>

Saving a few bytes with the added cost of harder to understand code.
Not really sure if that is worth it. I'll need to see a Tested-by:
for this patch.

Also, please fix the checkpatch warnings.

Thanks,
Guenter

> ---
>   drivers/hwmon/applesmc.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 0af63da6b603..0c950e1b03f3 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -84,12 +84,12 @@
>   #define TEMP_SENSOR_TYPE	"sp78"
>
>   /* List of keys used to read/write fan speeds */
> -static const char *const fan_speed_fmt[] = {
> -	"F%dAc",		/* actual speed */
> -	"F%dMn",		/* minimum speed (rw) */
> -	"F%dMx",		/* maximum speed */
> -	"F%dSf",		/* safe speed - not all models */
> -	"F%dTg",		/* target speed (manual: rw) */
> +static const char fan_speed_suffix[][4] = {
> +	"Ac",		/* actual speed */
> +	"Mn",		/* minimum speed (rw) */
> +	"Mx",		/* maximum speed */
> +	"Sf",		/* safe speed - not all models */
> +	"Tg",		/* target speed (manual: rw) */
>   };
>
>   #define INIT_TIMEOUT_MSECS	5000	/* wait up to 5s for device init ... */
> @@ -811,7 +811,7 @@ static ssize_t applesmc_show_fan_speed(struct device *dev,
>   	char newkey[5];
>   	u8 buffer[2];
>
> -	sprintf(newkey, fan_speed_fmt[to_option(attr)], to_index(attr));
> +	sprintf(newkey, "F%d%s", to_index(attr), fan_speed_suffix[to_option(attr)]);
>
>   	ret = applesmc_read_key(newkey, buffer, 2);
>   	speed = ((buffer[0] << 8 | buffer[1]) >> 2);
> @@ -834,7 +834,7 @@ static ssize_t applesmc_store_fan_speed(struct device *dev,
>   	if (kstrtoul(sysfsbuf, 10, &speed) < 0 || speed >= 0x4000)
>   		return -EINVAL;		/* Bigger than a 14-bit value */
>
> -	sprintf(newkey, fan_speed_fmt[to_option(attr)], to_index(attr));
> +	sprintf(newkey, "F%d%s", to_index(attr), fan_speed_suffix[to_option(attr)]);
>
>   	buffer[0] = (speed >> 6) & 0xff;
>   	buffer[1] = (speed << 2) & 0xff;
>

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