Re: [PATCH v1 1/1] treewide: Align match_string() with sysfs_match_string()
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-06-04 15:53:25
Also in:
alsa-devel, cgroups, dri-devel, intel-gfx, intel-xe, keyrings, linux-acpi, linux-bcachefs, linux-clk, linux-crypto, linux-fbdev, linux-gpio, linux-hardening, linux-hwmon, linux-i2c, linux-ide, linux-integrity, linux-leds, linux-mediatek, linux-mm, linux-mmc, linux-omap, linux-pci, linux-phy, linux-pm, linux-rockchip, linux-security-module, linux-sound, linux-staging, linux-sunxi, linux-tegra, linux-usb, linux-wireless, linuxppc-dev, lkml, netdev, nouveau, platform-driver-x86
On Tue, 04 Jun 2024 10:45:37 +0300
Jani Nikula [off-list ref] wrote:
On Sun, 02 Jun 2024, Andy Shevchenko [off-list ref] wrote:
quoted
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