[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3ojs6btxgava4dcasys5tnrg5vsrqlshagcg7otvrdgfcwwje4@lcrd3r6gkfcs>
Date: Tue, 4 Jun 2024 14:41:26 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Corey Minyard <minyard@....org>,
Allen Pais <apais@...ux.microsoft.com>, Sebastian Reichel <sebastian.reichel@...labora.com>,
Perry Yuan <perry.yuan@....com>, Giovanni Cabiddu <giovanni.cabiddu@...el.com>,
Herbert Xu <herbert@...dor.apana.org.au>, Nuno Sa <nuno.sa@...log.com>, Guenter Roeck <linux@...ck-us.net>,
Randy Dunlap <rdunlap@...radead.org>, Heiner Kallweit <hkallweit1@...il.com>,
Lee Jones <lee@...nel.org>, Samuel Holland <samuel@...lland.org>,
Elad Nachman <enachman@...vell.com>, Arseniy Krasnov <AVKrasnov@...rdevices.ru>,
Johannes Berg <johannes.berg@...el.com>, Gregory Greenman <gregory.greenman@...el.com>,
Benjamin Berg <benjamin.berg@...el.com>, Bjorn Helgaas <bhelgaas@...gle.com>,
Robert Richter <rrichter@....com>, Vinod Koul <vkoul@...nel.org>,
Chunfeng Yun <chunfeng.yun@...iatek.com>, Linus Walleij <linus.walleij@...aro.org>,
Hans de Goede <hdegoede@...hat.com>, Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Nikita Kravets <teackot@...il.com>, Jiri Slaby <jirislaby@...nel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, Stanley Chang <stanley_chang@...ltek.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Abdel Alkuor <abdelalkuor@...tab.com>,
Kent Overstreet <kent.overstreet@...ux.dev>, Eric Biggers <ebiggers@...gle.com>,
Kees Cook <keescook@...omium.org>, Ingo Molnar <mingo@...nel.org>,
"Steven Rostedt (Google)" <rostedt@...dmis.org>, Daniel Bristot de Oliveira <bristot@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>, Hugh Dickins <hughd@...gle.com>,
Abel Wu <wuyun.abel@...edance.com>, John Johansen <john.johansen@...onical.com>,
Mimi Zohar <zohar@...ux.ibm.com>, Stefan Berger <stefanb@...ux.ibm.com>,
Roberto Sassu <roberto.sassu@...wei.com>, Eric Snowberg <eric.snowberg@...cle.com>,
Takashi Iwai <tiwai@...e.de>, Takashi Sakamoto <o-takashi@...amocchi.jp>,
Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>, Mark Brown <broonie@...nel.org>,
Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>, linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
keyrings@...r.kernel.org, linux-crypto@...r.kernel.org, linux-acpi@...r.kernel.org,
linux-ide@...r.kernel.org, openipmi-developer@...ts.sourceforge.net,
linux-clk@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-tegra@...r.kernel.org,
linux-pm@...r.kernel.org, qat-linux@...el.com, dri-devel@...ts.freedesktop.org,
intel-gfx@...ts.freedesktop.org, intel-xe@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
linux-hwmon@...r.kernel.org, linux-i2c@...r.kernel.org, linux-leds@...r.kernel.org,
linux-sunxi@...ts.linux.dev, linux-omap@...r.kernel.org, linux-mmc@...r.kernel.org,
linux-mtd@...ts.infradead.org, netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
linux-pci@...r.kernel.org, linux-mediatek@...ts.infradead.org,
linux-phy@...ts.infradead.org, linux-gpio@...r.kernel.org, platform-driver-x86@...r.kernel.org,
linux-staging@...ts.linux.dev, linux-usb@...r.kernel.org, linux-fbdev@...r.kernel.org,
linux-bcachefs@...r.kernel.org, linux-hardening@...r.kernel.org, cgroups@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, linux-mm@...ck.org, apparmor@...ts.ubuntu.com,
linux-security-module@...r.kernel.org, linux-integrity@...r.kernel.org, alsa-devel@...a-project.org,
linux-sound@...r.kernel.org, Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>, Christophe Leroy <christophe.leroy@...roup.eu>,
"Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
David Howells <dhowells@...hat.com>, "David S. Miller" <davem@...emloft.net>,
"Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, Sergey Shtylyov <s.shtylyov@....ru>,
Damien Le Moal <dlemoal@...nel.org>, Niklas Cassel <cassel@...nel.org>,
Daniel Scally <djrscally@...il.com>, Sakari Ailus <sakari.ailus@...ux.intel.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
Florian Fainelli <florian.fainelli@...adcom.com>, Ray Jui <rjui@...adcom.com>,
Scott Branden <sbranden@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, Heiko Stuebner <heiko@...ech.de>,
Peter De Schrijver <pdeschrijver@...dia.com>, Prashant Gaikwad <pgaikwad@...dia.com>,
Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>,
Huang Rui <ray.huang@....com>, "Gautham R. Shenoy" <gautham.shenoy@....com>,
Mario Limonciello <mario.limonciello@....com>, Viresh Kumar <viresh.kumar@...aro.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, Jani Nikula <jani.nikula@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>, Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Tvrtko Ursulin <tursulin@...ulin.net>, Karol Herbst <kherbst@...hat.com>, Lyude Paul <lyude@...hat.com>,
Danilo Krummrich <dakr@...hat.com>, Jean Delvare <jdelvare@...e.com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Pavel Machek <pavel@....cz>, Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej.skrabec@...il.com>, Tony Lindgren <tony@...mide.com>,
Adrian Hunter <adrian.hunter@...el.com>, Hu Ziji <huziji@...vell.com>,
Ulf Hansson <ulf.hansson@...aro.org>, Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>,
Potnuri Bharat Teja <bharat@...lsio.com>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Miri Korenblit <miriam.rachel.korenblit@...el.com>, Kalle Valo <kvalo@...nel.org>,
Mahesh J Salgaonkar <mahesh@...ux.ibm.com>, Oliver O'Halloran <oohall@...il.com>,
Kishon Vijay Abraham I <kishon@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, JC Kuo <jckuo@...dia.com>, Andrew Lunn <andrew@...n.ch>,
Gregory Clement <gregory.clement@...tlin.com>, Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
Sebastian Reichel <sre@...nel.org>, Daniel Lezcano <daniel.lezcano@...aro.org>,
Zhang Rui <rui.zhang@...el.com>, Lukasz Luba <lukasz.luba@....com>,
Thinh Nguyen <Thinh.Nguyen@...opsys.com>, Helge Deller <deller@....de>, Brian Foster <bfoster@...hat.com>,
Zhihao Cheng <chengzhihao1@...wei.com>, Tejun Heo <tj@...nel.org>, Zefan Li <lizefan.x@...edance.com>,
Johannes Weiner <hannes@...xchg.org>, Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>, Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
Daniel Bristot de Oliveira <bristot@...hat.com>, Valentin Schneider <vschneid@...hat.com>,
Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Jason Baron <jbaron@...mai.com>, Jim Cromie <jim.cromie@...il.com>,
Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
"Serge E. Hallyn" <serge@...lyn.com>, Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
Clemens Ladisch <clemens@...isch.de>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Liam Girdwood <lgirdwood@...il.com>, Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v1 1/1] treewide: Align match_string() with
sysfs_match_string()
Hi Andy,
On Sun, Jun 02, 2024 at 06:57:12PM +0300, Andy Shevchenko 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.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
nice patch, I checked some (maybe most) of your changes. There
are a few unrelated changes which I don't mind, but there are two
errors where the error value changes from ENODEV to EINVAL.
Find the comments through the line.
...
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 1b7e82a0ad2e..b6f52f44625f 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1117,9 +1117,9 @@ static ssize_t store_energy_performance_preference(
> if (ret != 1)
> return -EINVAL;
>
> - ret = match_string(energy_perf_strings, -1, str_preference);
> + ret = __match_string(energy_perf_strings, -1, str_preference);
> if (ret < 0)
> - return -EINVAL;
> + return ret;
a bit of unrelated changes here, but I guess no one will complain :-)
>
> mutex_lock(&amd_pstate_limits_lock);
> ret = amd_pstate_set_energy_pref_index(cpudata, ret);
...
> diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c
> index cc9d28b75eb9..1865e26ae736 100644
> --- a/drivers/mmc/host/sdhci-xenon-phy.c
> +++ b/drivers/mmc/host/sdhci-xenon-phy.c
> @@ -135,15 +135,14 @@ struct xenon_emmc_phy_regs {
> u32 logic_timing_val;
> };
>
> -static const char * const phy_types[] = {
> - "emmc 5.0 phy",
> - "emmc 5.1 phy"
> -};
> -
> enum xenon_phy_type_enum {
> EMMC_5_0_PHY,
> EMMC_5_1_PHY,
> - NR_PHY_TYPES
> +};
> +
> +static const char * const phy_types[] = {
> + [EMMC_5_0_PHY] = "emmc 5.0 phy",
> + [EMMC_5_1_PHY] = "emmc 5.1 phy",
> };
Another unrelated cleanup, but I don't complain
> enum soc_pad_ctrl_type {
...
> - tablet_found = match_string(tablet_chassis_types,
> - ARRAY_SIZE(tablet_chassis_types),
> - chassis_type) >= 0;
> - if (!tablet_found)
> - return -ENODEV;
> + ret = match_string(tablet_chassis_types, chassis_type);
> + if (ret < 0)
> + return ret;
This is a logical change though, because we are changing from
-ENODEV to -EINVAL. Even if it might look the right thing, but
still, it's a logical change.
>
> ret = hp_wmi_perform_query(HPWMI_SYSTEM_DEVICE_MODE, HPWMI_READ,
> system_device_mode, zero_if_sup(system_device_mode),
> @@ -490,9 +487,7 @@ static bool is_omen_thermal_profile(void)
> if (!board_name)
> return false;
>
> - return match_string(omen_thermal_profile_boards,
> - ARRAY_SIZE(omen_thermal_profile_boards),
> - board_name) >= 0;
> + return match_string(omen_thermal_profile_boards, board_name) >= 0;
> }
>
> static int omen_get_thermal_policy_version(void)
...
> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
> index e56db75a94fb..dbd176b0fb1f 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
> @@ -111,7 +111,7 @@ static ssize_t suffix##_show(struct device *dev,\
> match_strs = (const char **)fivr_strings;\
> mmio_regs = tgl_fivr_mmio_regs;\
> } \
> - ret = match_string(match_strs, -1, attr->attr.name);\
> + ret = __match_string(match_strs, -1, attr->attr.name);\
> if (ret < 0)\
> return ret;\
> reg_val = readl((void __iomem *) (proc_priv->mmio_base + mmio_regs[ret].offset));\
> @@ -145,7 +145,7 @@ static ssize_t suffix##_store(struct device *dev,\
> mmio_regs = tgl_fivr_mmio_regs;\
> } \
> \
> - ret = match_string(match_strs, -1, attr->attr.name);\
> + ret = __match_string(match_strs, -1, attr->attr.name);\
> if (ret < 0)\
> return ret;\
> if (mmio_regs[ret].read_only)\
> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_wt_req.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_wt_req.c
> index f298e7442662..57f456befb34 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_wt_req.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_wt_req.c
> @@ -50,7 +50,7 @@ static ssize_t workload_type_store(struct device *dev,
> if (ret != 1)
> return -EINVAL;
>
> - ret = match_string(workload_types, -1, str_preference);
> + ret = __match_string(workload_types, -1, str_preference);
We could even thing of a "match_string_terminated" (or a better
name), but maybe it's too much?
> if (ret < 0)
> return ret;
>
...
> - c->auth_hash_algo = match_string(hash_algo_name, HASH_ALGO__LAST,
> - c->auth_hash_name);
> - if ((int)c->auth_hash_algo < 0) {
> + err = __match_string(hash_algo_name, HASH_ALGO__LAST, c->auth_hash_name);
> + if (err < 0) {
> ubifs_err(c, "Unknown hash algo %s specified",
> c->auth_hash_name);
> - return -EINVAL;
> + return err;
This is correct!
> }
> + c->auth_hash_algo = err;
>
> snprintf(hmac_name, CRYPTO_MAX_ALG_NAME, "hmac(%s)",
> c->auth_hash_name);
...
> +int __match_string(const char * const *array, size_t n, const char *string);
> +
> +/**
> + * match_string - matches given string in an array
> + * @_a: array of strings
> + * @_s: string to match with
> + *
> + * Helper for __match_string(). Calculates the size of @a automatically.
/@a/@_a/
> + */
> +#define match_string(_a, _s) __match_string(_a, ARRAY_SIZE(_a), _s)
> +
...
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 6239777090c4..e3fc94b4c7e5 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1820,9 +1820,9 @@ static int param_set_audit(const char *val, const struct kernel_param *kp)
> if (apparmor_initialized && !aa_current_policy_admin_capable(NULL))
> return -EPERM;
>
> - i = match_string(audit_mode_names, AUDIT_MAX_INDEX, val);
> + i = __match_string(audit_mode_names, AUDIT_MAX_INDEX, val);
pity here... this could have been a match_string, but the
MAX_INDEX is hardcoded outside the enum.
> if (i < 0)
> - return -EINVAL;
> + return i;
>
> aa_g_audit = i;
> return 0;
...
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 16dad4a45443..7064f4cae549 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -769,14 +769,13 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
> item = 0;
> }
>
> - i = match_string(e->texts, e->items, control_name);
> + i = __match_string(e->texts, e->items, control_name);
> if (i < 0)
> - return -ENODEV;
> + return i;
Also this return value is wrong.
Andi
>
> path->name = e->texts[i];
> path->connect = (i == item);
> return 0;
> -
> }
>
> /* set up initial codec paths */
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>
Powered by blists - more mailing lists