linux-kernel - Re: [PATCH] staging: iio: frequency: rename macros

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: <20241001225426.wUBOFdMi@linutronix.de>
Date: Wed, 2 Oct 2024 00:54:26 +0200
From: Nam Cao <namcao@...utronix.de>
To: Tudor Gheorghiu <tudor.reda@...il.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
	Michael Hennerich <Michael.Hennerich@...log.com>,
	Jonathan Cameron <jic23@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-iio@...r.kernel.org, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: iio: frequency: rename macros

On Tue, Oct 01, 2024 at 11:24:30PM +0300, Tudor Gheorghiu wrote:
> The frequency iio drivers use custom defined macros (inside dds.h) in
> order to define sysfs attributes more easily.
> 
> However, due to their naming choice and the fact that in some of them the
> first and/or second arguments are decimal, checkpatch will throw errors
> in the source files they are used in (ad9832.c and ad9834.c).
> This is because it thinks the argument is _mode, therefore
> it expects octal notation, even if the argument itself
> does not represent file permissions. Example:
> 
> ERROR: Use 4 digit octal (0777) not decimal permissions
> +static IIO_DEV_ATTR_PHASESYMBOL(0, 0200, NULL, ad9834_write, AD9834_PSEL);

You probably want to elaborate what you mean by "their naming choice" (i.e.
how does the naming choice causes this false warning?)

I got curious and digged into checkpatch.pl. This script expects macros
whose names match "IIO_DEV_ATTR_[A-Z_]+" to have the first integer argument
to be octal. And this driver defines macros which "luckily" match that
pattern.

There is only IIO_DEV_ATTR_SAMP_FREQ which matches the pattern, and accepts
umode_t as its first argument.

Instead of changing code just to make checkpatch.pl happy, perhaps it's
better to fix the checkpatch script? Maybe something like the untested
patch below?

Or since checkpatch is wrong, maybe just ignore it.

Best regards,
Nam

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4427572b2477..2fb4549fede2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -817,7 +817,7 @@ our @mode_permission_funcs = (
 	["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2],
 	["proc_create(?:_data|)", 2],
 	["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2],
-	["IIO_DEV_ATTR_[A-Z_]+", 1],
+	["IIO_DEV_ATTR_SAMP_FREQ", 1],
 	["SENSOR_(?:DEVICE_|)ATTR_2", 2],
 	["SENSOR_TEMPLATE(?:_2|)", 3],
 	["__ATTR", 2],