* [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data()
@ 2024-10-30 9:53 Dan Carpenter
2024-10-30 14:50 ` Andy Shevchenko
2024-11-01 14:10 ` Markus Elfring
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2024-10-30 9:53 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-iio
Hello Andy Shevchenko,
Commit 14686836fb69 ("iio: light: isl29018: Replace a variant of
iio_get_acpi_device_name_and_data()") from Oct 24, 2024 (linux-next),
leads to the following Smatch static checker warning:
drivers/iio/light/isl29018.c:724 isl29018_probe() error: uninitialized symbol 'ddata'.
drivers/iio/light/ltr501.c:1514 ltr501_probe() error: uninitialized symbol 'ddata'.
drivers/iio/light/isl29018.c
701 static int isl29018_probe(struct i2c_client *client)
702 {
703 const struct i2c_device_id *id = i2c_client_get_device_id(client);
704 struct isl29018_chip *chip;
705 struct iio_dev *indio_dev;
706 const void *ddata;
707 const char *name;
708 int dev_id;
709 int err;
710
711 indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
712 if (!indio_dev)
713 return -ENOMEM;
714
715 chip = iio_priv(indio_dev);
716
717 i2c_set_clientdata(client, indio_dev);
718
719 if (id) {
720 name = id->name;
721 dev_id = id->driver_data;
722 } else {
723 name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
--> 724 dev_id = (intptr_t)ddata;
How do we know that iio_get_acpi_device_name_and_data() will succeed?
725 }
726
727 mutex_init(&chip->lock);
728
729 chip->type = dev_id;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data()
2024-10-30 9:53 [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data() Dan Carpenter
@ 2024-10-30 14:50 ` Andy Shevchenko
2024-10-31 8:21 ` Andy Shevchenko
2024-11-01 14:10 ` Markus Elfring
1 sibling, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2024-10-30 14:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-iio
On Wed, Oct 30, 2024 at 12:53:57PM +0300, Dan Carpenter wrote:
> Hello Andy Shevchenko,
>
> Commit 14686836fb69 ("iio: light: isl29018: Replace a variant of
> iio_get_acpi_device_name_and_data()") from Oct 24, 2024 (linux-next),
> leads to the following Smatch static checker warning:
>
> drivers/iio/light/isl29018.c:724 isl29018_probe() error: uninitialized symbol 'ddata'.
> drivers/iio/light/ltr501.c:1514 ltr501_probe() error: uninitialized symbol 'ddata'.
>
> drivers/iio/light/isl29018.c
> 701 static int isl29018_probe(struct i2c_client *client)
> 702 {
> 703 const struct i2c_device_id *id = i2c_client_get_device_id(client);
> 704 struct isl29018_chip *chip;
> 705 struct iio_dev *indio_dev;
> 706 const void *ddata;
> 707 const char *name;
> 708 int dev_id;
> 709 int err;
> 710
> 711 indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> 712 if (!indio_dev)
> 713 return -ENOMEM;
> 714
> 715 chip = iio_priv(indio_dev);
> 716
> 717 i2c_set_clientdata(client, indio_dev);
> 718
> 719 if (id) {
> 720 name = id->name;
> 721 dev_id = id->driver_data;
> 722 } else {
> 723 name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
> --> 724 dev_id = (intptr_t)ddata;
>
> How do we know that iio_get_acpi_device_name_and_data() will succeed?
Ideally we need to file &ddata with NULL in such case, but it will be
equal to 0, so it only works with the chip_info in place.
Let me look into this once more, thanks for the good catch!
> 725 }
> 726
> 727 mutex_init(&chip->lock);
> 728
> 729 chip->type = dev_id;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data()
2024-10-30 14:50 ` Andy Shevchenko
@ 2024-10-31 8:21 ` Andy Shevchenko
2024-10-31 8:39 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2024-10-31 8:21 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-iio
On Wed, Oct 30, 2024 at 04:50:14PM +0200, Andy Shevchenko wrote:
> On Wed, Oct 30, 2024 at 12:53:57PM +0300, Dan Carpenter wrote:
> > Hello Andy Shevchenko,
> >
> > Commit 14686836fb69 ("iio: light: isl29018: Replace a variant of
> > iio_get_acpi_device_name_and_data()") from Oct 24, 2024 (linux-next),
> > leads to the following Smatch static checker warning:
> >
> > drivers/iio/light/isl29018.c:724 isl29018_probe() error: uninitialized symbol 'ddata'.
> > drivers/iio/light/ltr501.c:1514 ltr501_probe() error: uninitialized symbol 'ddata'.
> >
> > drivers/iio/light/isl29018.c
> > 701 static int isl29018_probe(struct i2c_client *client)
> > 702 {
> > 703 const struct i2c_device_id *id = i2c_client_get_device_id(client);
> > 704 struct isl29018_chip *chip;
> > 705 struct iio_dev *indio_dev;
> > 706 const void *ddata;
> > 707 const char *name;
> > 708 int dev_id;
> > 709 int err;
> > 710
> > 711 indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> > 712 if (!indio_dev)
> > 713 return -ENOMEM;
> > 714
> > 715 chip = iio_priv(indio_dev);
> > 716
> > 717 i2c_set_clientdata(client, indio_dev);
> > 718
> > 719 if (id) {
> > 720 name = id->name;
> > 721 dev_id = id->driver_data;
> > 722 } else {
> > 723 name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
> > --> 724 dev_id = (intptr_t)ddata;
> >
> > How do we know that iio_get_acpi_device_name_and_data() will succeed?
>
> Ideally we need to file &ddata with NULL in such case, but it will be
> equal to 0, so it only works with the chip_info in place.
>
> Let me look into this once more, thanks for the good catch!
I have sent a patch series to address that (you are in Cc there), does it help?
> > 725 }
> > 726
> > 727 mutex_init(&chip->lock);
> > 728
> > 729 chip->type = dev_id;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data()
2024-10-31 8:21 ` Andy Shevchenko
@ 2024-10-31 8:39 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2024-10-31 8:39 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-iio
On Thu, Oct 31, 2024 at 10:21:04AM +0200, Andy Shevchenko wrote:
> On Wed, Oct 30, 2024 at 04:50:14PM +0200, Andy Shevchenko wrote:
> > On Wed, Oct 30, 2024 at 12:53:57PM +0300, Dan Carpenter wrote:
> > > Hello Andy Shevchenko,
> > >
> > > Commit 14686836fb69 ("iio: light: isl29018: Replace a variant of
> > > iio_get_acpi_device_name_and_data()") from Oct 24, 2024 (linux-next),
> > > leads to the following Smatch static checker warning:
> > >
> > > drivers/iio/light/isl29018.c:724 isl29018_probe() error: uninitialized symbol 'ddata'.
> > > drivers/iio/light/ltr501.c:1514 ltr501_probe() error: uninitialized symbol 'ddata'.
> > >
> > > drivers/iio/light/isl29018.c
> > > 701 static int isl29018_probe(struct i2c_client *client)
> > > 702 {
> > > 703 const struct i2c_device_id *id = i2c_client_get_device_id(client);
> > > 704 struct isl29018_chip *chip;
> > > 705 struct iio_dev *indio_dev;
> > > 706 const void *ddata;
> > > 707 const char *name;
> > > 708 int dev_id;
> > > 709 int err;
> > > 710
> > > 711 indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> > > 712 if (!indio_dev)
> > > 713 return -ENOMEM;
> > > 714
> > > 715 chip = iio_priv(indio_dev);
> > > 716
> > > 717 i2c_set_clientdata(client, indio_dev);
> > > 718
> > > 719 if (id) {
> > > 720 name = id->name;
> > > 721 dev_id = id->driver_data;
> > > 722 } else {
> > > 723 name = iio_get_acpi_device_name_and_data(&client->dev, &ddata);
> > > --> 724 dev_id = (intptr_t)ddata;
> > >
> > > How do we know that iio_get_acpi_device_name_and_data() will succeed?
> >
> > Ideally we need to file &ddata with NULL in such case, but it will be
> > equal to 0, so it only works with the chip_info in place.
> >
> > Let me look into this once more, thanks for the good catch!
>
> I have sent a patch series to address that (you are in Cc there), does it help?
>
Yes. Thanks!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data()
2024-10-30 9:53 [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data() Dan Carpenter
2024-10-30 14:50 ` Andy Shevchenko
@ 2024-11-01 14:10 ` Markus Elfring
1 sibling, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2024-11-01 14:10 UTC (permalink / raw)
To: Dan Carpenter, linux-iio, Andy Shevchenko
Cc: LKML, Jonathan Cameron, Lars-Peter Clausen
> Commit 14686836fb69 ("iio: light: isl29018: Replace a variant of
> iio_get_acpi_device_name_and_data()") from Oct 24, 2024 (linux-next),
> leads to the following Smatch static checker warning:
>
> drivers/iio/light/isl29018.c:724 isl29018_probe() error: uninitialized symbol 'ddata'.
> drivers/iio/light/ltr501.c:1514 ltr501_probe() error: uninitialized symbol 'ddata'.
…
How do you think about to extend the mentioned source code analysis tool any more?
See also further development ideas:
* ltr501_probe()
https://lore.kernel.org/linux-kernel/8c7a5bcc-1fdb-45af-8f0c-1f9b6f0cf058@web.de/
https://lkml.org/lkml/2024/11/1/745
* isl29018_probe()
Is there a need to perform a null pointer check also with the local variable “name”
Regards,
Markus
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-01 14:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-30 9:53 [bug report] iio: light: isl29018: Replace a variant of iio_get_acpi_device_name_and_data() Dan Carpenter
2024-10-30 14:50 ` Andy Shevchenko
2024-10-31 8:21 ` Andy Shevchenko
2024-10-31 8:39 ` Dan Carpenter
2024-11-01 14:10 ` Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).