On Sun, 16 Nov 2025 15:35:12 +0000 Nam Cao <[email protected]> wrote:
> Convert to use __free to tidy up the code. > > Signed-off-by: Nam Cao <[email protected]> > --- > 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; > }
