linux-kernel - Re: [PATCH 3/3] iio: Add driver for Nicera D3-323-AA PIR sensor

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: <pndecwa85z5.fsf@axis.com>
Date: Tue, 27 May 2025 16:48:30 +0200
From: Waqar Hameed <waqar.hameed@...s.com>
To: Jonathan Cameron <jic23@...nel.org>
CC: Lars-Peter Clausen <lars@...afoo.de>, <kernel@...s.com>,
	<linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>
Subject: Re: [PATCH 3/3] iio: Add driver for Nicera D3-323-AA PIR sensor

On Sun, May 25, 2025 at 10:30 +0100 Jonathan Cameron <jic23@...nel.org> wrote:

> On Tue, 20 May 2025 13:27:54 +0200
> Waqar Hameed <waqar.hameed@...s.com> wrote:
>
>> On Sun, May 18, 2025 at 18:38 +0100 Jonathan Cameron <jic23@...nel.org> wrote:
>> 
>> >> >> +#define D3323AA_DRV_NAME "d3323aa"    
>> >> >
>> >> > Put that inline where used.  A define like this both implies that various values
>> >> > must be the same when they need not be and means that we have to go find the
>> >> > define to fine out what they are set to.  Just setting the strings directly
>> >> > tends to end up more readable.    
>> >> 
>> >> Sure, we can do that. (There are a bunch of IIO-drivers doing this, so I
>> >> just thought that was the "convention".)  
>> >
>> > I'm sometimes in less fussy mood.  One day I might just clean those up
>> > so there is nothing to copy into new drivers!  
>> 
>> A quick search tells that there are (at least) 105 of those:
>> 
>>   rgrep -A 30 "\.driver" drivers/iio/ | grep "\.name" | grep -v '"'
>>   
>> I was just about to write a small Python script to fix those, but just
>> wanted to confirm with you before spending more time on this. So if you
>> don't want to do this yourself, I can help your here :)
>
> It's probably not worth the churn on the ones that have the string repeated
> multiple times.  However, perhaps any that are only using it for .name would
> be good to tidy up?  Those are less a case of it being 'taste' vs it being silly
> to have a define!

I think if you use it in multiple places, it should definitively be a
macro definition. I just sent some patches for those that only used it
once (I didn't include those with `KBUILD_MODNAME`. We can discuss if we
should also address those in that thread).

However, there are a bunch of drivers that _only_ use a macro definition
in `.name` and `indio_dev->name`, including this one. That _is_ more
than one place, so we should actually leave it? Or do you still think we
should have the same string literal in both places, as you originally
commented above?