diff mbox series

leds: leds-wm831x-status: init chip_pdata before access

Message ID 20221122204837.11611-1-skhan@linuxfoundation.org
State New
Headers
Series leds: leds-wm831x-status: init chip_pdata before access |

Commit Message

Shuah Khan Nov. 22, 2022, 8:48 p.m. UTC 
  wm831x_status_probe() accesses status from chip_pdata before
initializing it. Fix it.

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 drivers/leds/leds-wm831x-status.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Pavel Machek Nov. 22, 2022, 9:05 p.m. UTC | #1 
Hi!

> wm831x_status_probe() accesses status from chip_pdata before
> initializing it. Fix it.
> 
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

Does it? ARRAY_SIZE() will be compile-time constant, no?

What is the bug? Did you test the code?

Best regards,
								Pavel


> +++ b/drivers/leds/leds-wm831x-status.c
> @@ -212,7 +212,7 @@ static int wm831x_status_probe(struct platform_device *pdev)
>  	struct wm831x_status_pdata pdata;
>  	struct wm831x_status *drvdata;
>  	struct resource *res;
> -	int id = pdev->id % ARRAY_SIZE(chip_pdata->status);
> +	int id;
>  	int ret;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_REG, 0);
> @@ -229,9 +229,10 @@ static int wm831x_status_probe(struct platform_device *pdev)
>  	drvdata->wm831x = wm831x;
>  	drvdata->reg = res->start;
>  
> -	if (dev_get_platdata(wm831x->dev))
> +	if (dev_get_platdata(wm831x->dev)) {
>  		chip_pdata = dev_get_platdata(wm831x->dev);
> -	else
> +		id = pdev->id % ARRAY_SIZE(chip_pdata->status);
> +	} else
>  		chip_pdata = NULL;
>  
>  	memset(&pdata, 0, sizeof(pdata));
Shuah Khan Nov. 22, 2022, 10:39 p.m. UTC | #2 
On 11/22/22 14:05, Pavel Machek wrote:
> Hi!
> 
>> wm831x_status_probe() accesses status from chip_pdata before
>> initializing it. Fix it.
>>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> 
> Does it? ARRAY_SIZE() will be compile-time constant, no?
> 
> What is the bug? Did you test the code?
> 

Is ARRAY_SIZE() safe when accessing the status chip_pdata->status?
I wasn't sure. If so, this change isn't necessary.

thanks,
-- Shuah
Pavel Machek Nov. 22, 2022, 11:08 p.m. UTC | #3 
On Tue 2022-11-22 15:39:35, Shuah Khan wrote:
> On 11/22/22 14:05, Pavel Machek wrote:
> > Hi!
> > 
> > > wm831x_status_probe() accesses status from chip_pdata before
> > > initializing it. Fix it.
> > > 
> > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> > 
> > Does it? ARRAY_SIZE() will be compile-time constant, no?
> > 
> > What is the bug? Did you test the code?
> > 
> 
> Is ARRAY_SIZE() safe when accessing the status chip_pdata->status?
> I wasn't sure. If so, this change isn't necessary.

I think so. Feel free to quite C standard to prove me wrong :-).

Best regards,
								Pavel
diff mbox series

Patch

diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
index c48b80574f02..5060c83f3b25 100644
--- a/drivers/leds/leds-wm831x-status.c
+++ b/drivers/leds/leds-wm831x-status.c
@@ -212,7 +212,7 @@  static int wm831x_status_probe(struct platform_device *pdev)
 	struct wm831x_status_pdata pdata;
 	struct wm831x_status *drvdata;
 	struct resource *res;
-	int id = pdev->id % ARRAY_SIZE(chip_pdata->status);
+	int id;
 	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_REG, 0);
@@ -229,9 +229,10 @@  static int wm831x_status_probe(struct platform_device *pdev)
 	drvdata->wm831x = wm831x;
 	drvdata->reg = res->start;
 
-	if (dev_get_platdata(wm831x->dev))
+	if (dev_get_platdata(wm831x->dev)) {
 		chip_pdata = dev_get_platdata(wm831x->dev);
-	else
+		id = pdev->id % ARRAY_SIZE(chip_pdata->status);
+	} else
 		chip_pdata = NULL;
 
 	memset(&pdata, 0, sizeof(pdata));