| Message ID | 20240228131406.165786-1-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath11k: fix few -Wmaybe-uninitialized warnings | expand |
Dmitry Antipov <dmantipov@yandex.ru> writes: > When compiling with gcc version 14.0.1 20240226 (experimental) and > W=12, I've noticed the following warnings: > > drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_remain_on_channel': > drivers/net/wireless/ath/ath11k/mac.c:9230:12: warning: 'ret' may be used uninitialized > [-Wmaybe-uninitialized] > 9230 | if (ret) > > drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem': > drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized > [-Wmaybe-uninitialized] > 2401 | return ret; > > drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi': > drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized > [-Wmaybe-uninitialized] > 2494 | release_firmware(fw_entry); > > And a bunch of them traced to uninitialized fields of the same > variable, e.g.: > > drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data': > drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may > be used uninitialized [-Wmaybe-uninitialized] > 700 | struct ath11k_spectral_summary_report summ_rpt; > > Fix all of the above by using 0 and NULL initializers where appropriate. > Note there are few more (less obvious) -Wmaybe-uninitialized warnings > still remains, but they're hardly possible to fix without running on > a physical hardware. Compile tested oly. > > Also noticed by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> Did you verify that the warnings are valid?
On 2/28/24 17:01, Kalle Valo wrote: > > Did you verify that the warnings are valid? > Well, hopefully yes. Just in case, gcc 13.2.1 issues an exactly the same set of -Wmaybe-uninitialized warnings. Dmitry
On 2/28/2024 5:14 AM, Dmitry Antipov wrote: > When compiling with gcc version 14.0.1 20240226 (experimental) and > W=12, I've noticed the following warnings: > > drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_remain_on_channel': > drivers/net/wireless/ath/ath11k/mac.c:9230:12: warning: 'ret' may be used uninitialized > [-Wmaybe-uninitialized] > 9230 | if (ret) > > drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem': > drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized > [-Wmaybe-uninitialized] > 2401 | return ret; > > drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi': > drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized > [-Wmaybe-uninitialized] > 2494 | release_firmware(fw_entry); > > And a bunch of them traced to uninitialized fields of the same > variable, e.g.: > > drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data': > drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may > be used uninitialized [-Wmaybe-uninitialized] > 700 | struct ath11k_spectral_summary_report summ_rpt; > > Fix all of the above by using 0 and NULL initializers where appropriate. > Note there are few more (less obvious) -Wmaybe-uninitialized warnings > still remains, but they're hardly possible to fix without running on > a physical hardware. Compile tested oly. > > Also noticed by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > --- > drivers/net/wireless/ath/ath11k/mac.c | 2 +- > drivers/net/wireless/ath/ath11k/qmi.c | 4 ++-- > drivers/net/wireless/ath/ath11k/spectral.c | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c > index a6a37d67a50a..b89bc7ceaaa7 100644 > --- a/drivers/net/wireless/ath/ath11k/mac.c > +++ b/drivers/net/wireless/ath/ath11k/mac.c > @@ -9201,7 +9201,7 @@ static int ath11k_mac_op_remain_on_channel(struct ieee80211_hw *hw, > struct ath11k *ar = hw->priv; > struct ath11k_vif *arvif = ath11k_vif_to_arvif(vif); > struct scan_req_params *arg; > - int ret; > + int ret = 0; NAK the only time ret would be uninitialized is if the initial switch() processed an unknown state, in which case we should stop processing, not process as successful. the correct fix for this is to add a default: to the switch() which warns and sets ret to an appropriate errno Please submit as a separate patch > u32 scan_time_msec; > > mutex_lock(&ar->conf_mutex); > diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c > index 5006f81f779b..4477f652e068 100644 > --- a/drivers/net/wireless/ath/ath11k/qmi.c > +++ b/drivers/net/wireless/ath/ath11k/qmi.c > @@ -2293,7 +2293,7 @@ static int ath11k_qmi_load_file_target_mem(struct ath11k_base *ab, > struct qmi_txn txn; > const u8 *temp = data; > void __iomem *bdf_addr = NULL; > - int ret; > + int ret = 0; I hate this since the only time ret would be uninitialized is if len is 0 (and hence we never execute the while(remaining) loop), but nothing ever calls this static function with len == 0 but the alternative to this is to add a guard check and that seems like overkill, so this one is ok > u32 remaining = len; > > req = kzalloc(sizeof(*req), GFP_KERNEL); > @@ -2406,7 +2406,7 @@ static int ath11k_qmi_load_bdf_qmi(struct ath11k_base *ab, > { > struct device *dev = ab->dev; > char filename[ATH11K_QMI_MAX_BDF_FILE_NAME_SIZE]; > - const struct firmware *fw_entry; > + const struct firmware *fw_entry = NULL; > struct ath11k_board_data bd; > u32 fw_size, file_type; > int ret = 0, bdf_type; > diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c > index 79e091134515..4c826b539404 100644 > --- a/drivers/net/wireless/ath/ath11k/spectral.c > +++ b/drivers/net/wireless/ath/ath11k/spectral.c > @@ -697,7 +697,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar, > struct ath11k_base *ab = ar->ab; > struct spectral_tlv *tlv; > struct spectral_summary_fft_report *summary = NULL; > - struct ath11k_spectral_summary_report summ_rpt; > + struct ath11k_spectral_summary_report summ_rpt = { 0 }; prefer just {} since that works correctly if we ever want to add a non-scalar as the first member > struct fft_sample_ath11k *fft_sample = NULL; > u8 *data; > u32 data_len, i;
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index a6a37d67a50a..b89bc7ceaaa7 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -9201,7 +9201,7 @@ static int ath11k_mac_op_remain_on_channel(struct ieee80211_hw *hw, struct ath11k *ar = hw->priv; struct ath11k_vif *arvif = ath11k_vif_to_arvif(vif); struct scan_req_params *arg; - int ret; + int ret = 0; u32 scan_time_msec; mutex_lock(&ar->conf_mutex); diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c index 5006f81f779b..4477f652e068 100644 --- a/drivers/net/wireless/ath/ath11k/qmi.c +++ b/drivers/net/wireless/ath/ath11k/qmi.c @@ -2293,7 +2293,7 @@ static int ath11k_qmi_load_file_target_mem(struct ath11k_base *ab, struct qmi_txn txn; const u8 *temp = data; void __iomem *bdf_addr = NULL; - int ret; + int ret = 0; u32 remaining = len; req = kzalloc(sizeof(*req), GFP_KERNEL); @@ -2406,7 +2406,7 @@ static int ath11k_qmi_load_bdf_qmi(struct ath11k_base *ab, { struct device *dev = ab->dev; char filename[ATH11K_QMI_MAX_BDF_FILE_NAME_SIZE]; - const struct firmware *fw_entry; + const struct firmware *fw_entry = NULL; struct ath11k_board_data bd; u32 fw_size, file_type; int ret = 0, bdf_type; diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c index 79e091134515..4c826b539404 100644 --- a/drivers/net/wireless/ath/ath11k/spectral.c +++ b/drivers/net/wireless/ath/ath11k/spectral.c @@ -697,7 +697,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar, struct ath11k_base *ab = ar->ab; struct spectral_tlv *tlv; struct spectral_summary_fft_report *summary = NULL; - struct ath11k_spectral_summary_report summ_rpt; + struct ath11k_spectral_summary_report summ_rpt = { 0 }; struct fft_sample_ath11k *fft_sample = NULL; u8 *data; u32 data_len, i;
When compiling with gcc version 14.0.1 20240226 (experimental) and W=12, I've noticed the following warnings: drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_remain_on_channel': drivers/net/wireless/ath/ath11k/mac.c:9230:12: warning: 'ret' may be used uninitialized [-Wmaybe-uninitialized] 9230 | if (ret) drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_file_target_mem': drivers/net/wireless/ath/ath11k/qmi.c:2401:16: warning: 'ret' may be used uninitialized [-Wmaybe-uninitialized] 2401 | return ret; drivers/net/wireless/ath/ath11k/qmi.c: In function 'ath11k_qmi_load_bdf_qmi': drivers/net/wireless/ath/ath11k/qmi.c:2494:17: warning: 'fw_entry' may be used uninitialized [-Wmaybe-uninitialized] 2494 | release_firmware(fw_entry); And a bunch of them traced to uninitialized fields of the same variable, e.g.: drivers/net/wireless/ath/ath11k/spectral.c: In function 'ath11k_spectral_process_data': drivers/net/wireless/ath/ath11k/spectral.c:700:47: warning: 'summ_rpt.meta.freq1' may be used uninitialized [-Wmaybe-uninitialized] 700 | struct ath11k_spectral_summary_report summ_rpt; Fix all of the above by using 0 and NULL initializers where appropriate. Note there are few more (less obvious) -Wmaybe-uninitialized warnings still remains, but they're hardly possible to fix without running on a physical hardware. Compile tested oly. Also noticed by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- drivers/net/wireless/ath/ath11k/mac.c | 2 +- drivers/net/wireless/ath/ath11k/qmi.c | 4 ++-- drivers/net/wireless/ath/ath11k/spectral.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)