* Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> wrote: > On Fri, 26 Jan 2024 at 22:05, Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > > > > Committer: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > > > CommitterDate: Fri, 26 Jan 2024 13:44:48 +01:00 > > > > > > genirq/irq_sim: Shrink code by using cleanup helpers > > > > > > Use the new __free() mechanism to remove all gotos and simplify the error > > > paths. > > > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > > > Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > > > Link: https://lore.kernel.org/r/20240122124243.44002-5-brgl@xxxxxxxx > > > > > > --- > > > kernel/irq/irq_sim.c | 25 ++++++++++--------------- > > > 1 file changed, 10 insertions(+), 15 deletions(-) > > > > > > diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c > > > index b0d50b4..fe8fd30 100644 > > > --- a/kernel/irq/irq_sim.c > > > +++ b/kernel/irq/irq_sim.c > > > @@ -4,6 +4,7 @@ > > > * Copyright (C) 2020 Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx> > > > */ > > > > > > +#include <linux/cleanup.h> > > > #include <linux/interrupt.h> > > > #include <linux/irq.h> > > > #include <linux/irq_sim.h> > > > @@ -163,33 +164,27 @@ static const struct irq_domain_ops irq_sim_domain_ops = { > > > struct irq_domain *irq_domain_create_sim(struct fwnode_handle *fwnode, > > > unsigned int num_irqs) > > > { > > > - struct irq_sim_work_ctx *work_ctx; > > > + struct irq_sim_work_ctx *work_ctx __free(kfree) = kmalloc(sizeof(*work_ctx), GFP_KERNEL); > > > + unsigned long *pending; > > > > > > - work_ctx = kmalloc(sizeof(*work_ctx), GFP_KERNEL); > > > if (!work_ctx) > > > - goto err_out; > > > + return ERR_PTR(-ENOMEM); > > > > > > - work_ctx->pending = bitmap_zalloc(num_irqs, GFP_KERNEL); > > > - if (!work_ctx->pending) > > > - goto err_free_work_ctx; > > > + pending = __free(bitmap) = bitmap_zalloc(num_irqs, GFP_KERNEL); > > > > Apologies if this has already been reported elsewhere. This does not > > match what was sent and it causes the build to break with both GCC: > > > > I did not see any other report. I don't know what happened here but > this was a ninja edit as it's not what I sent. If Thomas' intention > was to move the variable declaration and detach it from the assignment > then 'pending' should at least be set to NULL and __free() must > decorate the declaration. > > But the coding style of declaring variables when they're first > assigned their auto-cleaned value is what Linus Torvalds explicitly > asked me to do when I first started sending PRs containing uses of > linux/cleanup.h. Ok - I've rebased tip:irq/core with the original patch. Do you have a reference to Linus's mail about C++ style definition of variables? I can see the validity of the pattern in this context, but it's explicitly against the kernel coding style AFAICS, which I suppose prompted Thomas's edit. I'd like to have an URL handy when the inevitable checkpatch 'fix' gets submitted. ;-) Thanks, Ingo