linux-kernel - Re: [PATCH v1 02/13] iio: chemical: bme680: avoid using camel case

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: <Zwj3QZE30juE53Vy@smile.fi.intel.com>
Date: Fri, 11 Oct 2024 13:00:33 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: vamoirid <vassilisamir@...il.com>
Cc: jic23@...nel.org, lars@...afoo.de, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, anshulusr@...il.com, gustavograzs@...il.com,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 02/13] iio: chemical: bme680: avoid using camel case

On Thu, Oct 10, 2024 at 11:00:19PM +0200, vamoirid wrote:
> From: Vasileios Amoiridis <vassilisamir@...il.com>
> 
> Rename camel case variable, as checkpatch.pl complains.

With given reply to the first patch...

...

>  	/* Look up table for the possible gas range values */
> -	static const u32 lookupTable[16] = {2147483647u, 2147483647u,
> +	static const u32 lookup_table[16] = {2147483647u, 2147483647u,
>  				2147483647u, 2147483647u, 2147483647u,
>  				2126008810u, 2147483647u, 2130303777u,
>  				2147483647u, 2147483647u, 2143188679u,

...here is the opportunity to fix indentation while at fixing the code.
I.o.w. I would reformat the entire table to be

	static const u32 lookup_table[16] = {
		2147483647u, 2147483647u, 2147483647u, 2147483647u,
		2147483647u, 2126008810u, 2147483647u, 2130303777u,
		2147483647u, 2147483647u, 2143188679u, ...

(also note power-of-2 number of items per line which much easier to read and
 find one you need).

...

>  	var1 = ((1340 + (5 * (s64)calib->range_sw_err)) *
> -			((s64)lookupTable[gas_range])) >> 16;
> +			((s64)lookup_table[gas_range])) >> 16;

Also an opportunity to make this neater like

	var1 = (1340 + (5 * (s64)calib->range_sw_err)) * (s64)lookup_table[gas_range]);
	var1 >>= 16;

So, at bare minumym there are redundant parentheses. And looking at the table
and the first argument of multiplication I'm puzzled why casting is needed for
the second? Shouldn't s64 already be implied by the first one?

-- 
With Best Regards,
Andy Shevchenko