On Fri, 23 May 2014, Sam Ravnborg wrote:
> 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" <davem@xxxxxxxxxxxxx>,sparclinux@xxxxxxxxxxxxxxx,linux-kernel@xxxxxxxxxxxxxxx
> >
> > 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.
I don't see it. It looks to me that it returned ENOMEM, due to the
assignment after the kzalloc.
julia
> > }
> >
> > 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 sparclinux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
