* [PATCH] drm/panel: himax-hx8279: Initialize goa_{even,odd}_valid in hx8279_check_goa_config()
@ 2025-04-22 21:20 Nathan Chancellor
2025-04-23 13:45 ` Neil Armstrong
0 siblings, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2025-04-22 21:20 UTC (permalink / raw)
To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: Jessica Zhang, AngeloGioacchino Del Regno, dri-devel, llvm,
patches, Nathan Chancellor
Clang warns (or errors with CONFIG_WERROR=y):
drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use occurs here
842 | if (goa_odd_valid != goa_even_valid)
| ^~~~~~~~~~~~~~
drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if its condition is always true
838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
839 | goa_even_valid = false;
drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the variable 'goa_even_valid' to silence this warning
818 | bool goa_odd_valid, goa_even_valid;
| ^
| = 0
Even though only the even valid variable gets flagged, both valid
variables appear to have the same issue of possibly being used
uninitialized if the if statement initializing them to false is not
taken.
Initialize both values to true to clear up the warning and remove any
possibility of encountering undefined behavior.
Fixes: 38d42c261389 ("drm: panel: Add driver for Himax HX8279 DDIC panels")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/gpu/drm/panel/panel-himax-hx8279.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-himax-hx8279.c b/drivers/gpu/drm/panel/panel-himax-hx8279.c
index b48b350b62da..92f351e66c25 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx8279.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx8279.c
@@ -815,7 +815,7 @@ static int hx8279_check_gmux_config(struct hx8279 *hx, struct device *dev)
static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
{
const struct hx8279_panel_desc *desc = hx->desc;
- bool goa_odd_valid, goa_even_valid;
+ bool goa_odd_valid = true, goa_even_valid = true;
int i, num_zero, num_clr = 0;
/* Up to 4 zero values is a valid configuration. Check them all. */
---
base-commit: dcbd5dcc956e2331414fd7020b4655df08deeb87
change-id: 20250422-panel-himax-hx8279-fix-sometimes-uninitialized-207354fb930c
Best regards,
--
Nathan Chancellor <nathan@kernel.org>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drm/panel: himax-hx8279: Initialize goa_{even,odd}_valid in hx8279_check_goa_config()
2025-04-22 21:20 [PATCH] drm/panel: himax-hx8279: Initialize goa_{even,odd}_valid in hx8279_check_goa_config() Nathan Chancellor
@ 2025-04-23 13:45 ` Neil Armstrong
2025-04-23 13:50 ` Neil Armstrong
0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2025-04-23 13:45 UTC (permalink / raw)
To: Nathan Chancellor, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: Jessica Zhang, AngeloGioacchino Del Regno, dri-devel, llvm,
patches
On 22/04/2025 23:20, Nathan Chancellor wrote:
> Clang warns (or errors with CONFIG_WERROR=y):
>
> drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
> 838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use occurs here
> 842 | if (goa_odd_valid != goa_even_valid)
> | ^~~~~~~~~~~~~~
> drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if its condition is always true
> 838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 839 | goa_even_valid = false;
> drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the variable 'goa_even_valid' to silence this warning
> 818 | bool goa_odd_valid, goa_even_valid;
> | ^
> | = 0
>
> Even though only the even valid variable gets flagged, both valid
> variables appear to have the same issue of possibly being used
> uninitialized if the if statement initializing them to false is not
> taken.
>
> Initialize both values to true to clear up the warning and remove any
> possibility of encountering undefined behavior.
>
> Fixes: 38d42c261389 ("drm: panel: Add driver for Himax HX8279 DDIC panels")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> drivers/gpu/drm/panel/panel-himax-hx8279.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-himax-hx8279.c b/drivers/gpu/drm/panel/panel-himax-hx8279.c
> index b48b350b62da..92f351e66c25 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx8279.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx8279.c
> @@ -815,7 +815,7 @@ static int hx8279_check_gmux_config(struct hx8279 *hx, struct device *dev)
> static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
> {
> const struct hx8279_panel_desc *desc = hx->desc;
> - bool goa_odd_valid, goa_even_valid;
> + bool goa_odd_valid = true, goa_even_valid = true;
> int i, num_zero, num_clr = 0;
>
> /* Up to 4 zero values is a valid configuration. Check them all. */
>
> ---
> base-commit: dcbd5dcc956e2331414fd7020b4655df08deeb87
> change-id: 20250422-panel-himax-hx8279-fix-sometimes-uninitialized-207354fb930c
>
> Best regards,
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/panel: himax-hx8279: Initialize goa_{even,odd}_valid in hx8279_check_goa_config()
2025-04-23 13:45 ` Neil Armstrong
@ 2025-04-23 13:50 ` Neil Armstrong
2025-04-23 14:01 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2025-04-23 13:50 UTC (permalink / raw)
To: Nathan Chancellor, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: Jessica Zhang, AngeloGioacchino Del Regno, dri-devel, llvm,
patches
On 23/04/2025 15:45, Neil Armstrong wrote:
> On 22/04/2025 23:20, Nathan Chancellor wrote:
>> Clang warns (or errors with CONFIG_WERROR=y):
>>
>> drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>> 838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use occurs here
>> 842 | if (goa_odd_valid != goa_even_valid)
>> | ^~~~~~~~~~~~~~
>> drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if its condition is always true
>> 838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 839 | goa_even_valid = false;
>> drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the variable 'goa_even_valid' to silence this warning
>> 818 | bool goa_odd_valid, goa_even_valid;
>> | ^
>> | = 0
>>
>> Even though only the even valid variable gets flagged, both valid
>> variables appear to have the same issue of possibly being used
>> uninitialized if the if statement initializing them to false is not
>> taken.
>>
>> Initialize both values to true to clear up the warning and remove any
>> possibility of encountering undefined behavior.
>>
>> Fixes: 38d42c261389 ("drm: panel: Add driver for Himax HX8279 DDIC panels")
>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>> ---
>> drivers/gpu/drm/panel/panel-himax-hx8279.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-himax-hx8279.c b/drivers/gpu/drm/panel/panel-himax-hx8279.c
>> index b48b350b62da..92f351e66c25 100644
>> --- a/drivers/gpu/drm/panel/panel-himax-hx8279.c
>> +++ b/drivers/gpu/drm/panel/panel-himax-hx8279.c
>> @@ -815,7 +815,7 @@ static int hx8279_check_gmux_config(struct hx8279 *hx, struct device *dev)
>> static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
>> {
>> const struct hx8279_panel_desc *desc = hx->desc;
>> - bool goa_odd_valid, goa_even_valid;
>> + bool goa_odd_valid = true, goa_even_valid = true;
>> int i, num_zero, num_clr = 0;
>> /* Up to 4 zero values is a valid configuration. Check them all. */
>>
>> ---
>> base-commit: dcbd5dcc956e2331414fd7020b4655df08deeb87
>> change-id: 20250422-panel-himax-hx8279-fix-sometimes-uninitialized-207354fb930c
>>
>> Best regards,
>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
I'll wait a few days until AngeloGioacchino Del Regno reviews it to be sure
it's the right fix.
Thanks,
Neil
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/panel: himax-hx8279: Initialize goa_{even,odd}_valid in hx8279_check_goa_config()
2025-04-23 13:50 ` Neil Armstrong
@ 2025-04-23 14:01 ` AngeloGioacchino Del Regno
2025-04-23 17:29 ` Nathan Chancellor
0 siblings, 1 reply; 5+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-04-23 14:01 UTC (permalink / raw)
To: neil.armstrong, Nathan Chancellor, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann
Cc: Jessica Zhang, dri-devel, llvm, patches
Il 23/04/25 15:50, Neil Armstrong ha scritto:
> On 23/04/2025 15:45, Neil Armstrong wrote:
>> On 22/04/2025 23:20, Nathan Chancellor wrote:
>>> Clang warns (or errors with CONFIG_WERROR=y):
>>>
>>> drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable
>>> 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-
>>> Werror,-Wsometimes-uninitialized]
>>> 838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use
>>> occurs here
>>> 842 | if (goa_odd_valid != goa_even_valid)
>>> | ^~~~~~~~~~~~~~
>>> drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if
>>> its condition is always true
>>> 838 | if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> 839 | goa_even_valid = false;
>>> drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the
>>> variable 'goa_even_valid' to silence this warning
>>> 818 | bool goa_odd_valid, goa_even_valid;
>>> | ^
>>> | = 0
>>>
>>> Even though only the even valid variable gets flagged, both valid
>>> variables appear to have the same issue of possibly being used
>>> uninitialized if the if statement initializing them to false is not
>>> taken.
>>>
>>> Initialize both values to true to clear up the warning and remove any
>>> possibility of encountering undefined behavior.
>>>
>>> Fixes: 38d42c261389 ("drm: panel: Add driver for Himax HX8279 DDIC panels")
>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>> ---
>>> drivers/gpu/drm/panel/panel-himax-hx8279.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-himax-hx8279.c b/drivers/gpu/drm/panel/
>>> panel-himax-hx8279.c
>>> index b48b350b62da..92f351e66c25 100644
>>> --- a/drivers/gpu/drm/panel/panel-himax-hx8279.c
>>> +++ b/drivers/gpu/drm/panel/panel-himax-hx8279.c
>>> @@ -815,7 +815,7 @@ static int hx8279_check_gmux_config(struct hx8279 *hx,
>>> struct device *dev)
>>> static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
>>> {
>>> const struct hx8279_panel_desc *desc = hx->desc;
>>> - bool goa_odd_valid, goa_even_valid;
>>> + bool goa_odd_valid = true, goa_even_valid = true;
>>> int i, num_zero, num_clr = 0;
>>> /* Up to 4 zero values is a valid configuration. Check them all. */
>>>
>>> ---
>>> base-commit: dcbd5dcc956e2331414fd7020b4655df08deeb87
>>> change-id: 20250422-panel-himax-hx8279-fix-sometimes-uninitialized-207354fb930c
>>>
>>> Best regards,
>>
>> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
>
> I'll wait a few days until AngeloGioacchino Del Regno reviews it to be sure
> it's the right fix.
>
> Thanks,
> Neil
I would prefer
if (num_zero == ARRAY_SIZE(desc->goa_odd_timing))
goa_odd_valid = false;
else
goa_odd_valid = true;
/* Up to 3 zeroes is a valid config. Check them all. */
num_zero = 1;
for (i = 0; i < ARRAY_SIZE(desc->goa_even_timing); i++) {
if (desc->goa_even_timing[i])
num_zero++;
}
if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
goa_even_valid = false;
else
goa_even_valid = true;
or the shorter form:
goa_odd_valid = (num_zero != ARRAY_SIZE(desc->goa_odd_timing));
/* Up to 3 zeroes is a valid config. Check them all. */
num_zero = 1;
for (i = 0; i < ARRAY_SIZE(desc->goa_even_timing); i++) {
if (desc->goa_even_timing[i])
num_zero++;
}
goa_even_valid = (num_zero != ARRAY_SIZE(desc->goa_even_timing));
...to avoid double initialization.
But anyway, the proposed solution also works just fine, as the sense is, as well
understood by Nathan, "if it's all zeroes something is wrong, otherwise it's ok".
Btw , I'm sorry about this - not sure how the code ended up being like this,
probably that last minute cleanup that I did.
Ultimately, whether to avoid the double init or not is Neil's call - so for this
commit you can get my
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
And sorry again about the (stupid) mistake.
Cheers,
Angelo
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/panel: himax-hx8279: Initialize goa_{even,odd}_valid in hx8279_check_goa_config()
2025-04-23 14:01 ` AngeloGioacchino Del Regno
@ 2025-04-23 17:29 ` Nathan Chancellor
0 siblings, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2025-04-23 17:29 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: neil.armstrong, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jessica Zhang, dri-devel, llvm, patches
On Wed, Apr 23, 2025 at 04:01:20PM +0200, AngeloGioacchino Del Regno wrote:
> or the shorter form:
>
> goa_odd_valid = (num_zero != ARRAY_SIZE(desc->goa_odd_timing));
>
> /* Up to 3 zeroes is a valid config. Check them all. */
> num_zero = 1;
> for (i = 0; i < ARRAY_SIZE(desc->goa_even_timing); i++) {
> if (desc->goa_even_timing[i])
> num_zero++;
> }
>
> goa_even_valid = (num_zero != ARRAY_SIZE(desc->goa_even_timing));
>
Ah yeah, I think I like this the best as it is the most compact while
always ensuring the variable is always initialized. I will send a v2
with your Reviewed-by carried forward and an additional Suggested-by
shortly.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-23 17:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22 21:20 [PATCH] drm/panel: himax-hx8279: Initialize goa_{even,odd}_valid in hx8279_check_goa_config() Nathan Chancellor
2025-04-23 13:45 ` Neil Armstrong
2025-04-23 13:50 ` Neil Armstrong
2025-04-23 14:01 ` AngeloGioacchino Del Regno
2025-04-23 17:29 ` Nathan Chancellor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox