| Message ID | 20180724071223.18901-1-daniel.thompson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] backlight: pwm_bl: Fix uninitialized variable | expand |
Hi, On Tue, Jul 24, 2018 at 12:12 AM, Daniel Thompson <daniel.thompson@linaro.org> wrote: > Currently, if the DT does not define num-interpolated-steps then > num_steps is undefined and the interpolation code will deploy randomly. > Fix with a simple initialize to zero. > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation between > brightness-levels") I think it may be incorrect to word-wrap the Fixes line. A quick grep through the source code looks like others agree. > Reported-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > --- > > Notes: > v3: > - Switch to the simplest fix and zero the uninitialized variable. git > grep indicates that ~25% of calls to of_property_read_u32() use this > pattern (pre-initialize optional properties with sane values and > ignore the return code). > > v2: > - Simplify SoB chain (with Marcel's permission) > - Separate complex if statement and make other similar calls use same > return code checking approach > - Tidy up comment formatting and fix pre-existing grammar error > > drivers/video/backlight/pwm_bl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Other than the nit on the commit message, this simple fix seems sane. Reviewed-by: Douglas Anderson <dianders@chromium.org>
On Tue, 24 Jul 2018, Daniel Thompson wrote: > Currently, if the DT does not define num-interpolated-steps then > num_steps is undefined and the interpolation code will deploy randomly. > Fix with a simple initialize to zero. > > Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation between > brightness-levels") > Reported-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > --- > > Notes: > v3: > - Switch to the simplest fix and zero the uninitialized variable. git > grep indicates that ~25% of calls to of_property_read_u32() use this > pattern (pre-initialize optional properties with sane values and > ignore the return code). > > v2: > - Simplify SoB chain (with Marcel's permission) > - Separate complex if statement and make other similar calls use same > return code checking approach > - Tidy up comment formatting and fix pre-existing grammar error > > drivers/video/backlight/pwm_bl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Once the 'Fixes:' line has been repaired: Acked-by: Lee Jones <lee.jones@linaro.org>
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 9ee4c1b735b2..bdfcc0a71db1 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -250,7 +250,7 @@ static int pwm_backlight_parse_dt(struct device *dev, struct device_node *node = dev->of_node; unsigned int num_levels = 0; unsigned int levels_count; - unsigned int num_steps; + unsigned int num_steps = 0; struct property *prop; unsigned int *table; int length;
Currently, if the DT does not define num-interpolated-steps then num_steps is undefined and the interpolation code will deploy randomly. Fix with a simple initialize to zero. Fixes: 573fe6d1c25c ("backlight: pwm_bl: Linear interpolation between brightness-levels") Reported-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> --- Notes: v3: - Switch to the simplest fix and zero the uninitialized variable. git grep indicates that ~25% of calls to of_property_read_u32() use this pattern (pre-initialize optional properties with sane values and ignore the return code). v2: - Simplify SoB chain (with Marcel's permission) - Separate complex if statement and make other similar calls use same return code checking approach - Tidy up comment formatting and fix pre-existing grammar error drivers/video/backlight/pwm_bl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.17.1