Hi Himangi. On Fri, May 23, 2014 at 02:05:36AM +0530, Himangi Saraogi wrote: > This patch moves data allocated using kzalloc to managed data allocated > using devm_kzalloc and cleans now unnecessary kfrees in probe and remove > functions. The header file is added to make the devm function explicitly > available. Also, unnecessary assignment statements and labels are > removed. > > --- > Not compile tested due to incompatible architecture. I can confirm that i builds.
> > To send to: "David S. Miller" > <[email protected]>,[email protected],[email protected] > > drivers/sbus/char/display7seg.c | 23 ++++++++--------------- > 1 file changed, 8 insertions(+), 15 deletions(-) > > diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c > index 7c71e7b..367b64d 100644 > --- a/drivers/sbus/char/display7seg.c > +++ b/drivers/sbus/char/display7seg.c > @@ -20,6 +20,7 @@ > #include <asm/io.h> > > #include <asm/display7seg.h> > +#include <linux/device.h> > > #define D7S_MINOR 193 > #define DRIVER_NAME "d7s" > @@ -173,22 +174,21 @@ static struct miscdevice d7s_miscdev = { > static int d7s_probe(struct platform_device *op) > { > struct device_node *opts; > - int err = -EINVAL; > + int err; > struct d7s *p; > u8 regs; > > if (d7s_device) > - goto out; > + return -EINVAL; > > - p = kzalloc(sizeof(*p), GFP_KERNEL); > - err = -ENOMEM; > + p = devm_kzalloc(&op->dev, sizeof(*p), GFP_KERNEL); > if (!p) > - goto out; > + return -ENOMEM; > > p->regs = of_ioremap(&op->resource[0], 0, sizeof(u8), "d7s"); > if (!p->regs) { > printk(KERN_ERR PFX "Cannot map chip registers\n"); > - goto out_free; > + return -ENOMEM; Can you please explain how it can be correct to return "-ENOMEM", when we fail to map the chip registers. Before your patch this resulted in "-EINVAL" - so this is a change in behaviour. > } > > err = misc_register(&d7s_miscdev); > @@ -222,17 +222,11 @@ static int d7s_probe(struct platform_device *op) > > dev_set_drvdata(&op->dev, p); > d7s_device = p; > - err = 0; > - > -out: > - return err; > + return 0; > > out_iounmap: > of_iounmap(&op->resource[0], p->regs, sizeof(u8)); > - > -out_free: > - kfree(p); > - goto out; > + return err; > } It is not am improvement IMO that you replace the nice goto based error handling with random return points in the code. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
