Re: [PATCH v1 1/1] treewide: Align match_string() with sysfs_match_string()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 04 Jun 2024 10:45:37 +0300
Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:

> On Sun, 02 Jun 2024, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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.

(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.

-- Steve





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux