linux-kernel - Re: [PATCH 1/3] iio: dummy: convert all simple allocation 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]

Date:   Mon, 7 Dec 2020 09:22:28 +0200
From:   Alexandru Ardelean <ardeleanalex@...il.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Alexandru Ardelean <alexandru.ardelean@...log.com>,
        linux-iio <linux-iio@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants

On Sat, Dec 5, 2020 at 8:46 PM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Thu, 3 Dec 2020 11:50:03 +0200
> Alexandru Ardelean <alexandru.ardelean@...log.com> wrote:
>
> > Since a main requirement for an IIO device is to have a parent device
> > object, it makes sense to attach more of the IIO device's objects to the
> > lifetime of the parent object, to clean up the code.
> > The idea is to also provide a base example that is more up-to-date with
> > what's going on lately with most IIO drivers.
>
> To some degree maybe, it's also very very handy for testing odd corner
> cases with.  I'd definitely not recommend it as a reference driver
> because it inherently has some odd corners because we need to
> fake various things that don't exist without hardware.

It's funny because during GSoC it seemed like some people would try to
use this as a starting point, then shift to another working ADC/DAC
example.
I was also thinking about maybe extending this a bit further, to have
it a bit more dynamic at load time [being able to load fake IIO
drivers, load fake data to be read via fake IIO device].
No idea when this will be ready, but it's on my long todo-list.

>
> >
> > This change tackles the simple allocations, to convert them to
> > device-managed calls, and tie them to the parent device.
>
> I'm confused or maybe I missrecall how this works.
>
> IIRC there isn't a parent device...
>
> Maybe we could create one via a bit of smoke and magic.

Yep, there isn't one.
I'll add one through some smoke, magic, some OF and maybe some fake
i2c/spi device IDs.

>
>
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
> > ---
> >  drivers/iio/dummy/iio_simple_dummy.c | 29 ++++++++--------------------
> >  1 file changed, 8 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
> > index c0b7ef900735..2a2e62f780a1 100644
> > --- a/drivers/iio/dummy/iio_simple_dummy.c
> > +++ b/drivers/iio/dummy/iio_simple_dummy.c
> > @@ -574,11 +574,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> >        * parent = &client->dev;
> >        */
> >
> > -     swd = kzalloc(sizeof(*swd), GFP_KERNEL);
> > -     if (!swd) {
> > -             ret = -ENOMEM;
> > -             goto error_kzalloc;
> > -     }
> > +     swd = devm_kzalloc(parent, sizeof(*swd), GFP_KERNEL);
> > +     if (!swd)
> > +             return ERR_PTR(-ENOMEM);
> >       /*
> >        * Allocate an IIO device.
> >        *
> > @@ -587,11 +585,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> >        * It also has a region (accessed by iio_priv()
> >        * for chip specific state information.
> >        */
> > -     indio_dev = iio_device_alloc(parent, sizeof(*st));
> > -     if (!indio_dev) {
> > -             ret = -ENOMEM;
> > -             goto error_ret;
> > -     }
> > +     indio_dev = devm_iio_device_alloc(parent, sizeof(*st));
> > +     if (!indio_dev)
> > +             return ERR_PTR(-ENOMEM);
> >
> >       st = iio_priv(indio_dev);
> >       mutex_init(&st->lock);
> > @@ -615,7 +611,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> >        *    indio_dev->name = id->name;
> >        *    indio_dev->name = spi_get_device_id(spi)->name;
> >        */
> > -     indio_dev->name = kstrdup(name, GFP_KERNEL);
> > +     indio_dev->name = devm_kstrdup(parent, name, GFP_KERNEL);
> >
> >       /* Provide description of available channels */
> >       indio_dev->channels = iio_dummy_channels;
> > @@ -632,7 +628,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> >
> >       ret = iio_simple_dummy_events_register(indio_dev);
> >       if (ret < 0)
> > -             goto error_free_device;
> > +             return ERR_PTR(ret);
> >
> >       ret = iio_simple_dummy_configure_buffer(indio_dev);
> >       if (ret < 0)
> > @@ -649,11 +645,6 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> >       iio_simple_dummy_unconfigure_buffer(indio_dev);
> >  error_unregister_events:
> >       iio_simple_dummy_events_unregister(indio_dev);
> > -error_free_device:
> > -     iio_device_free(indio_dev);
> > -error_ret:
> > -     kfree(swd);
> > -error_kzalloc:
> >       return ERR_PTR(ret);
> >  }
> >
> > @@ -683,10 +674,6 @@ static int iio_dummy_remove(struct iio_sw_device *swd)
> >
> >       iio_simple_dummy_events_unregister(indio_dev);
> >
> > -     /* Free all structures */
> > -     kfree(indio_dev->name);
> > -     iio_device_free(indio_dev);
> > -
> >       return 0;
> >  }
> >
>