| Message ID | 20241030160756.2099326-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: acpi: always initialise data in iio_get_acpi_device_name_and_data() | expand |
On Wed, 30 Oct 2024 18:02:17 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Fill data with NULL, if provided, when returning NULL from > iio_get_acpi_device_name_and_data(). Note, the current users check > for name to be valid, except one case which was initially doing > like that and has to be fixed separately. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/r/54fac4a7-b601-40ce-8c00-d94807f5e214@stanley.mountain > Fixes: dc60de4eb0a4 ("iio: acpi: Add iio_get_acpi_device_name_and_data() helper function") > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> This is counter intuitive as usual expectation would be no side effects on an error return. How hard to fix all the users to initialize to NULL if they care about that? There is still a chance we set it to NULL in here anyway, but that should only happen if we know the return is good in the sense of no error (missing ACPI etc) but not necessarily that dev_name() won't return NULL. Don't think dev_name() can currently return NULL but 'maybe' it could... > --- > drivers/iio/industrialio-acpi.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-acpi.c b/drivers/iio/industrialio-acpi.c > index d67a43843799..33c737c7a2f8 100644 > --- a/drivers/iio/industrialio-acpi.c > +++ b/drivers/iio/industrialio-acpi.c > @@ -102,13 +102,17 @@ EXPORT_SYMBOL_GPL(iio_read_acpi_mount_matrix); > * NOTE: This helper function exists only for backward compatibility, > * do not use in a new code! > * > - * Returns: ACPI device instance name or %NULL. > + * Returns: ACPI device instance name or %NULL When returning %NULL, the @data, > + * if provided, will be also initialised to %NULL. > */ > const char *iio_get_acpi_device_name_and_data(struct device *dev, const void **data) > { > const struct acpi_device_id *id; > acpi_handle handle; > > + if (data) > + *data = NULL; > + > handle = ACPI_HANDLE(dev); > if (!handle) > return NULL;
On Thu, Oct 31, 2024 at 07:17:17PM +0000, Jonathan Cameron wrote: > On Wed, 30 Oct 2024 18:02:17 +0200 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > Fill data with NULL, if provided, when returning NULL from > > iio_get_acpi_device_name_and_data(). Note, the current users check > > for name to be valid, except one case which was initially doing > > like that and has to be fixed separately. > > > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > > Closes: https://lore.kernel.org/r/54fac4a7-b601-40ce-8c00-d94807f5e214@stanley.mountain > > Fixes: dc60de4eb0a4 ("iio: acpi: Add iio_get_acpi_device_name_and_data() helper function") > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > This is counter intuitive as usual expectation would be no side effects on an > error return. How hard to fix all the users to initialize to NULL if they > care about that? v2 just has been sent, indeed the result looks much better, thanks for the review! > There is still a chance we set it to NULL in here anyway, but that should only happen > if we know the return is good in the sense of no error (missing ACPI etc) > but not necessarily that dev_name() won't return NULL. > > Don't think dev_name() can currently return NULL but 'maybe' it could... dev_name() can't return NULL on the properly initialised device (either with device_add(), or via dev_set_name() call). I do not think we can ever get to the ->probe() without the above. Tell me, if I'm wrong.
diff --git a/drivers/iio/industrialio-acpi.c b/drivers/iio/industrialio-acpi.c index d67a43843799..33c737c7a2f8 100644 --- a/drivers/iio/industrialio-acpi.c +++ b/drivers/iio/industrialio-acpi.c @@ -102,13 +102,17 @@ EXPORT_SYMBOL_GPL(iio_read_acpi_mount_matrix); * NOTE: This helper function exists only for backward compatibility, * do not use in a new code! * - * Returns: ACPI device instance name or %NULL. + * Returns: ACPI device instance name or %NULL When returning %NULL, the @data, + * if provided, will be also initialised to %NULL. */ const char *iio_get_acpi_device_name_and_data(struct device *dev, const void **data) { const struct acpi_device_id *id; acpi_handle handle; + if (data) + *data = NULL; + handle = ACPI_HANDLE(dev); if (!handle) return NULL;
Fill data with NULL, if provided, when returning NULL from iio_get_acpi_device_name_and_data(). Note, the current users check for name to be valid, except one case which was initially doing like that and has to be fixed separately. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/r/54fac4a7-b601-40ce-8c00-d94807f5e214@stanley.mountain Fixes: dc60de4eb0a4 ("iio: acpi: Add iio_get_acpi_device_name_and_data() helper function") Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/iio/industrialio-acpi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)