| Message ID | 20220803115720.89848-1-matt.ranostay@konsulko.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: add NULL pointer checks to iio device additional/removal | expand |
On Wed, Aug 3, 2022 at 2:04 PM Matt Ranostay <matt.ranostay@konsulko.com> wrote: > > Check if __iio_device_register and iio_device_unregister indio_dev > parameter isn't a NULL pointer. We refer to functions as func() in the text. ... > int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod) > { > - struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); > + struct iio_dev_opaque *iio_dev_opaque; > struct fwnode_handle *fwnode; > int ret; > > - if (!indio_dev->info) > + if (!indio_dev || !indio_dev->info) > return -EINVAL; > > + iio_dev_opaque = to_iio_dev_opaque(indio_dev); Not sure we need this one. All callers check for indio_dev from the allocator, otherwise I don't see a clean workflow when indio_dev is NULL when calling register(). The other part is good.
On Wed, Aug 3, 2022 at 7:57 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, Aug 3, 2022 at 2:04 PM Matt Ranostay <matt.ranostay@konsulko.com> wrote: > > > > Check if __iio_device_register and iio_device_unregister indio_dev > > parameter isn't a NULL pointer. A couple more things: - make sure you include all maintainers and reviewers (you may use my "smart" script [1] for that) - explain why you are doing this change [1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
On Wed, 3 Aug 2022 20:00:24 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, Aug 3, 2022 at 7:57 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Wed, Aug 3, 2022 at 2:04 PM Matt Ranostay <matt.ranostay@konsulko.com> wrote: > > > > > > Check if __iio_device_register and iio_device_unregister indio_dev > > > parameter isn't a NULL pointer. > > A couple more things: > - make sure you include all maintainers and reviewers (you may use my > "smart" script [1] for that) > - explain why you are doing this change > Agreed. I'm not sure of the reasoning for doing this. I'd also expect to see it as part of a series where we might pass NULL to these functions. Thanks, Jonathan > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh >
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 0f4dbda3b9d3..6071e52903e5 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -1896,13 +1896,14 @@ static const struct iio_buffer_setup_ops noop_ring_setup_ops; int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod) { - struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + struct iio_dev_opaque *iio_dev_opaque; struct fwnode_handle *fwnode; int ret; - if (!indio_dev->info) + if (!indio_dev || !indio_dev->info) return -EINVAL; + iio_dev_opaque = to_iio_dev_opaque(indio_dev); iio_dev_opaque->driver_module = this_mod; /* If the calling driver did not initialize firmware node, do it here */ @@ -1987,7 +1988,12 @@ EXPORT_SYMBOL(__iio_device_register); **/ void iio_device_unregister(struct iio_dev *indio_dev) { - struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + struct iio_dev_opaque *iio_dev_opaque; + + if (!indio_dev) + return; + + iio_dev_opaque = to_iio_dev_opaque(indio_dev); cdev_device_del(&iio_dev_opaque->chrdev, &indio_dev->dev);
Check if __iio_device_register and iio_device_unregister indio_dev parameter isn't a NULL pointer. Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com> --- drivers/iio/industrialio-core.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)