Received-SPF: pass (google.com: domain of linux-kernel+bounces-201087-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Tue, 4 Jun 2024 19:36:22 +0300
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Jani Nikula <jani.nikula@linux.intel.com>, linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v1 1/1] treewide: Align match_string() with
 sysfs_match_string()
Message-ID: <Zl9ChheV8lS8_DUd@smile.fi.intel.com>
References: <20240603211538.289765-1-andriy.shevchenko@linux.intel.com>
 <87tti9cfry.fsf@intel.com>
 <20240604115235.044acfd6@gandalf.local.home>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240604115235.044acfd6@gandalf.local.home>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Tue, Jun 04, 2024 at 11:52:35AM -0400, Steven Rostedt wrote:
> On Tue, 04 Jun 2024 10:45:37 +0300
> Jani Nikula <jani.nikula@linux.intel.com> wrote:
> 
> > On Sun, 02 Jun 2024, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > Make two APIs look similar. Hence convert match_string() to be
> > > a 2-argument macro. In order to avoid unneeded churn, convert
> > > all users as well. There is no functional change intended.  
> > 
> > Why do we think it's a good idea to increase and normalize the use of
> > double-underscore function names across the kernel, like
> > __match_string() in this case? It should mean "reserved for the
> > implementation, not to be called directly".
> > 
> > If it's to be used directly, it should be named accordingly, right?
> > 
> > Being in line with __sysfs_match_string() isn't a great argument alone,
> > because this adds three times the number of __match_string() calls than
> > there are __sysfs_match_string() calls. It's not a good model to follow.
> > Arguably both should be renamed.
> 
> Agreed. I want to get rid of any functions starting with an underscore
> except for those that are basically the same function used internally for
> convenience.
> 
> Perhaps "match_string_dynamic()"? Where it is used for dynamically
> allocated arrays without known size. Or, allow a third parameter for
> dynamic arrays.
> 
> #define match_string(_a, _s, ...)
> 	char _______STR[] = __stringify((__VA_ARGS__));	\
> 	if (sizeof(_______STR) > 3)			\
> 		__match_string(_a, _s, ##__VA_ARGS__);  \
> 	else						\
> 		__match_string(_a, _s, ARRAY_SIZE(_a));
> 
> What the above stringify((__VA_ARGS__)) does is to check the size of any
> args added to match_string(). if there isn't any, it will turn into:
> "()\0", which is of size 3. If you add an argument, it will be:
> "(<arg>)\0", which will have a size greater than three.

We have args.h for such tricks.

> (trace_printk() does this trick in include/linux/kernel.h).
> 
> This way, both:
> 
>  match_string(array, sting);
> 
> or
> 
>  match_string(array, string, size);
> 
> will work.

But Linus already gave a few good advice including the name. I don't wont to
switch to this magic for match_string() as it's fine to have a simple macro.

-- 
With Best Regards,
Andy Shevchenko