[PATCH] ath10k: avoid -Wmaybe-uninitialized warning

[Arnd Bergmann]

In some configurations the inlining in gcc is suboptimal, causing
a false-positive warning:

drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_mac_init_rd':
drivers/net/wireless/ath/ath10k/mac.c:8374:39: error: 'rd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
ar->ath_common.regulatory.current_rd = rd;

If we initialize the output of ath10k_mac_get_wrdd_regulatory()
before returning, this problem goes away.

Fixes: 209b2a68de76 ("ath10k: add platform regulatory domain support")
Signed-off-by: Arnd Bergmann <***@arndb.de>
---
drivers/net/wireless/ath/ath10k/mac.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index a1c2801ded10..0d5fde28ee44 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -8321,6 +8321,8 @@ static int ath10k_mac_get_wrdd_regulatory(struct ath10k *ar, u16 *rd)
u32 alpha2_code;
char alpha2[3];

+ *rd = ar->hw_eeprom_rd;
+
root_handle = ACPI_HANDLE(&pdev->dev);
if (!root_handle)
return -EOPNOTSUPP;
@@ -8365,11 +8367,9 @@ static int ath10k_mac_init_rd(struct ath10k *ar)
u16 rd;

ret = ath10k_mac_get_wrdd_regulatory(ar, &rd);
- if (ret) {
+ if (ret)
ath10k_dbg(ar, ATH10K_DBG_BOOT,
"fallback to eeprom programmed regulatory settings\n");
- rd = ar->hw_eeprom_rd;
- }

ar->ath_common.regulatory.current_rd = rd;
return 0;

[Brian Norris]

Hi,

Post by Arnd Bergmann
In some configurations the inlining in gcc is suboptimal, causing
drivers/net/wireless/ath/ath10k/mac.c:8374:39: error: 'rd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
ar->ath_common.regulatory.current_rd = rd;
If we initialize the output of ath10k_mac_get_wrdd_regulatory()
before returning, this problem goes away.
Fixes: 209b2a68de76 ("ath10k: add platform regulatory domain support")
---
drivers/net/wireless/ath/ath10k/mac.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index a1c2801ded10..0d5fde28ee44 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -8321,6 +8321,8 @@ static int ath10k_mac_get_wrdd_regulatory(struct ath10k *ar, u16 *rd)
u32 alpha2_code;
char alpha2[3];
+ *rd = ar->hw_eeprom_rd;
+

Maybe it's just me, but it seems kinda weird for this function to
assign a (valid) value to its "output" and still potentially return an
error.

If you really need to work around this compiler bug, maybe just put
the eeprom assignment back in ath10k_mac_init_rd()? I'll leave it up
to Kalle as to whether he wants to work around the compiler at all :)

Oh wait, one more thing: this is actually an invalid refactoring. See
how this function assigns '*rd' later in error cases. Today, we still
treat those as errors and clobber those with the eeprom value, but
now, you're making the fallback case continue to use the erroneous
value (0xffff). You need to make that use a local variable and avoid
clobbering *rd, if you want this to be correct.

Post by Arnd Bergmann
root_handle = ACPI_HANDLE(&pdev->dev);

Side note: your patch made me notice -- this code is *not* only used
on PCI devices, yet it utilizes the 'pci_dev' structure. Fortunately,
it only does this to needless convert it right back to a bare
'device', and then the ACPI code safely handles non-ACPI devices
(should result in -EOPNOTSUPP below), but this is awfully strange
code.

Anyway, I'm just thinking out loud. I'll probably patch the pci_dev out myself.

Brian

Post by Arnd Bergmann
if (!root_handle)
return -EOPNOTSUPP;
@@ -8365,11 +8367,9 @@ static int ath10k_mac_init_rd(struct ath10k *ar)
u16 rd;
ret = ath10k_mac_get_wrdd_regulatory(ar, &rd);
- if (ret) {
+ if (ret)
ath10k_dbg(ar, ATH10K_DBG_BOOT,
"fallback to eeprom programmed regulatory settings\n");
- rd = ar->hw_eeprom_rd;
- }
ar->ath_common.regulatory.current_rd = rd;
return 0;
--
2.18.0

[Kalle Valo]

Post by Brian Norris
Hi,

Post by Arnd Bergmann
In some configurations the inlining in gcc is suboptimal, causing
drivers/net/wireless/ath/ath10k/mac.c:8374:39: error: 'rd' may be used uninitialized in this function [-Werror=maybe-uninitialized]
ar->ath_common.regulatory.current_rd = rd;
If we initialize the output of ath10k_mac_get_wrdd_regulatory()
before returning, this problem goes away.
Fixes: 209b2a68de76 ("ath10k: add platform regulatory domain support")
---
drivers/net/wireless/ath/ath10k/mac.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index a1c2801ded10..0d5fde28ee44 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -8321,6 +8321,8 @@ static int ath10k_mac_get_wrdd_regulatory(struct ath10k *ar, u16 *rd)
u32 alpha2_code;
char alpha2[3];
+ *rd = ar->hw_eeprom_rd;
+

Maybe it's just me, but it seems kinda weird for this function to
assign a (valid) value to its "output" and still potentially return an
error.
If you really need to work around this compiler bug, maybe just put
the eeprom assignment back in ath10k_mac_init_rd()? I'll leave it up
to Kalle as to whether he wants to work around the compiler at all :)

In general I'm happy take workaround to compiler problems, I prefer to
keep ath10k warning free much as possible.

Post by Brian Norris
Oh wait, one more thing: this is actually an invalid refactoring. See
how this function assigns '*rd' later in error cases. Today, we still
treat those as errors and clobber those with the eeprom value, but
now, you're making the fallback case continue to use the erroneous
value (0xffff). You need to make that use a local variable and avoid
clobbering *rd, if you want this to be correct.

But I agree with Brian here, I don't think this patch is correct.