| 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
| ||
|
Message-ID: <20251117100735.71466d12@gandalf.local.home>
Date: Mon, 17 Nov 2025 10:07:35 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Nam Cao <namcao@...utronix.de>
Cc: Gabriele Monaco <gmonaco@...hat.com>, Masami Hiramatsu
<mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] rv: Convert to use __free
On Sun, 16 Nov 2025 15:35:12 +0000
Nam Cao <namcao@...utronix.de> wrote:
> Convert to use __free to tidy up the code.
>
> Signed-off-by: Nam Cao <namcao@...utronix.de>
> ---
> kernel/trace/rv/rv.c | 65 +++++++++++++++--------------------
> kernel/trace/rv/rv.h | 6 ++--
> kernel/trace/rv/rv_reactors.c | 32 ++++++++---------
> 3 files changed, 46 insertions(+), 57 deletions(-)
>
> diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
> index b1059a3cf4fa..e83dc9f0c7bb 100644
> --- a/kernel/trace/rv/rv.c
> +++ b/kernel/trace/rv/rv.c
> @@ -420,35 +420,28 @@ static const struct file_operations interface_desc_fops = {
> static int create_monitor_dir(struct rv_monitor *mon, struct rv_monitor *parent)
> {
> struct dentry *root = parent ? parent->root_d : get_monitors_root();
> - const char *name = mon->name;
> + struct dentry *dir __free(rv_remove) = rv_create_dir(mon->name, root);
> struct dentry *tmp;
> int retval;
>
> - mon->root_d = rv_create_dir(name, root);
> - if (!mon->root_d)
> + if (!dir)
> return -ENOMEM;
>
> - tmp = rv_create_file("enable", RV_MODE_WRITE, mon->root_d, mon, &interface_enable_fops);
> - if (!tmp) {
> - retval = -ENOMEM;
> - goto out_remove_root;
> - }
> + tmp = rv_create_file("enable", RV_MODE_WRITE, dir, mon, &interface_enable_fops);
> + if (!tmp)
> + return -ENOMEM;
>
> - tmp = rv_create_file("desc", RV_MODE_READ, mon->root_d, mon, &interface_desc_fops);
> - if (!tmp) {
> - retval = -ENOMEM;
> - goto out_remove_root;
> - }
> + tmp = rv_create_file("desc", RV_MODE_READ, dir, mon, &interface_desc_fops);
> + if (!tmp)
> + return -ENOMEM;
>
> - retval = reactor_populate_monitor(mon);
> + retval = reactor_populate_monitor(mon, dir);
> if (retval)
> - goto out_remove_root;
> + return retval;
>
> + mon->root_d = dir;
> + retain_and_null_ptr(dir);
> return 0;
Why the "retain_and_null_ptr() and not just:
mon->root-d = no_free_ptr(dir);
return 0;
As from my understanding is that the retain_and_null_ptr() is for use of
passing the variable to a function that will consume it. But for assigning
to a variable, I usually just use the no_free_ptr().
> -
> -out_remove_root:
> - rv_remove(mon->root_d);
> - return retval;
> }
>
> /*
> @@ -827,39 +820,37 @@ int __init rv_init_interface(void)
> {
> struct dentry *tmp;
> int retval;
> + struct dentry *root_dir __free(rv_remove) = rv_create_dir("rv", NULL);
>
> - rv_root.root_dir = rv_create_dir("rv", NULL);
> - if (!rv_root.root_dir)
> - goto out_err;
> + if (!root_dir)
> + return 1;
>
> - rv_root.monitors_dir = rv_create_dir("monitors", rv_root.root_dir);
> + rv_root.monitors_dir = rv_create_dir("monitors", root_dir);
> if (!rv_root.monitors_dir)
> - goto out_err;
> + return 1;
>
> - tmp = rv_create_file("available_monitors", RV_MODE_READ, rv_root.root_dir, NULL,
> + tmp = rv_create_file("available_monitors", RV_MODE_READ, root_dir, NULL,
> &available_monitors_ops);
> if (!tmp)
> - goto out_err;
> + return 1;
>
> - tmp = rv_create_file("enabled_monitors", RV_MODE_WRITE, rv_root.root_dir, NULL,
> + tmp = rv_create_file("enabled_monitors", RV_MODE_WRITE, root_dir, NULL,
> &enabled_monitors_ops);
> if (!tmp)
> - goto out_err;
> + return 1;
>
> - tmp = rv_create_file("monitoring_on", RV_MODE_WRITE, rv_root.root_dir, NULL,
> + tmp = rv_create_file("monitoring_on", RV_MODE_WRITE, root_dir, NULL,
> &monitoring_on_fops);
> if (!tmp)
> - goto out_err;
> - retval = init_rv_reactors(rv_root.root_dir);
> + return 1;
> + retval = init_rv_reactors(root_dir);
> if (retval)
> - goto out_err;
> + return 1;
>
> turn_monitoring_on();
>
> - return 0;
> + rv_root.root_dir = root_dir;
> + retain_and_null_ptr(root_dir);
Same here.
>
> -out_err:
> - rv_remove(rv_root.root_dir);
> - printk(KERN_ERR "RV: Error while creating the RV interface\n");
> - return 1;
> + return 0;
> }
> @@ -438,30 +439,25 @@ static struct rv_reactor rv_nop = {
>
> int init_rv_reactors(struct dentry *root_dir)
> {
> - struct dentry *available, *reacting;
> int retval;
>
> - available = rv_create_file("available_reactors", RV_MODE_READ, root_dir, NULL,
> - &available_reactors_ops);
> - if (!available)
> - goto out_err;
> + struct dentry *available __free(rv_remove) =
> + rv_create_file("available_reactors", RV_MODE_READ, root_dir,
> + NULL, &available_reactors_ops);
>
> - reacting = rv_create_file("reacting_on", RV_MODE_WRITE, root_dir, NULL, &reacting_on_fops);
> - if (!reacting)
> - goto rm_available;
> + struct dentry *reacting =
> + rv_create_file("reacting_on", RV_MODE_WRITE, root_dir, NULL, &reacting_on_fops);
> +
> + if (!reacting || !available)
> + return -ENOMEM;
>
> retval = __rv_register_reactor(&rv_nop);
> if (retval)
> - goto rm_reacting;
> + return retval;
>
> turn_reacting_on();
>
> + retain_and_null_ptr(available);
> + retain_and_null_ptr(reacting);
Here it makes sense to use retain_and_null_ptr() as the two variables were
passed to another function. But the other two locations should simply use
the no_free_ptr() wrapper.
-- Steve
> return 0;
> -
> -rm_reacting:
> - rv_remove(reacting);
> -rm_available:
> - rv_remove(available);
> -out_err:
> - return -ENOMEM;
> }
Powered by blists - more mailing lists