linux-kernel - Re: [PATCH] USB: apple-mfi-fastcharge: Use devm

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <X6/xhYwdw/RPBXf9@kroah.com>
Date:   Sat, 14 Nov 2020 16:02:29 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Lucas Tanure <tanure@...ux.com>
Cc:     Bastien Nocera <hadess@...ess.net>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB: apple-mfi-fastcharge: Use devm_kzalloc and simplify
 the code

On Sat, Nov 14, 2020 at 02:17:48PM +0000, Lucas Tanure wrote:
> On Sat, Nov 14, 2020 at 12:56 PM Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
> >
> > On Sat, Nov 14, 2020 at 12:42:49PM +0000, Lucas Tanure wrote:
> > > Signed-off-by: Lucas Tanure <tanure@...ux.com>
> >
> > I can't take patches without any changelog text, sorry.
> >
> > > ---
> > >  drivers/usb/misc/apple-mfi-fastcharge.c | 17 +++++------------
> > >  1 file changed, 5 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/usb/misc/apple-mfi-fastcharge.c b/drivers/usb/misc/apple-mfi-fastcharge.c
> > > index 9de0171b5177..de86e389a008 100644
> > > --- a/drivers/usb/misc/apple-mfi-fastcharge.c
> > > +++ b/drivers/usb/misc/apple-mfi-fastcharge.c
> > > @@ -178,16 +178,13 @@ static int mfi_fc_probe(struct usb_device *udev)
> > >  {
> > >       struct power_supply_config battery_cfg = {};
> > >       struct mfi_device *mfi = NULL;
> > > -     int err;
> > >
> > >       if (!mfi_fc_match(udev))
> > >               return -ENODEV;
> > >
> > > -     mfi = kzalloc(sizeof(struct mfi_device), GFP_KERNEL);
> > > -     if (!mfi) {
> > > -             err = -ENOMEM;
> > > -             goto error;
> > > -     }
> > > +     mfi = devm_kzalloc(&udev->dev, sizeof(*mfi), GFP_KERNEL);
> > > +     if (!mfi)
> > > +             return -ENOMEM;
> > >
> > >       battery_cfg.drv_data = mfi;
> > >
> > > @@ -197,8 +194,7 @@ static int mfi_fc_probe(struct usb_device *udev)
> > >                                               &battery_cfg);
> > >       if (IS_ERR(mfi->battery)) {
> > >               dev_err(&udev->dev, "Can't register battery\n");
> > > -             err = PTR_ERR(mfi->battery);
> > > -             goto error;
> > > +             return PTR_ERR(mfi->battery);
> > >       }
> > >
> > >       mfi->udev = usb_get_dev(udev);
> > > @@ -206,9 +202,6 @@ static int mfi_fc_probe(struct usb_device *udev)
> > >
> > >       return 0;
> > >
> > > -error:
> > > -     kfree(mfi);
> > > -     return err;
> > >  }
> > >
> > >  static void mfi_fc_disconnect(struct usb_device *udev)
> > > @@ -220,7 +213,7 @@ static void mfi_fc_disconnect(struct usb_device *udev)
> > >               power_supply_unregister(mfi->battery);
> > >       dev_set_drvdata(&udev->dev, NULL);
> > >       usb_put_dev(mfi->udev);
> > > -     kfree(mfi);
> > > +     devm_kfree(&udev->dev, mfi);
> >
> > Are you sure about this?
> I think so, as the probe will allocate again that struct, the
> disconnect should free the previous one.

Why do you need to manually free it here like this?

Why are you trying to convert this file to this api anyway?

thanks,

greg k-h