| Message ID | 20240229084031.51957-3-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [1/3,v2] wifi: ath11k: use ath11k_mac_get_ar_by_pdev_id() consistently | expand |
On 2/29/2024 12:40 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/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 an 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] looks like a false positive since ath11k_spectral_pull_summary() always sets the entire meta struct: memcpy(&report->meta, meta, sizeof(*meta)); > 700 | struct ath11k_spectral_summary_report summ_rpt; > > Fix all of the above by using 0, NULL, and {} initializers, respectively. > 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 only. > > Also noticed by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > --- > v2: use {} initializer (Jeff Johnson) and aggregate to the series > --- > drivers/net/wireless/ath/ath11k/qmi.c | 4 ++-- > drivers/net/wireless/ath/ath11k/spectral.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > 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; as previously discussed this one I'll begrudgingly take. please submit as a stand-alone patch > 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; note that this also seems to be fixing a false positive. the "maybe uninitialized" reference is: out_qmi_cal: if (!ab->qmi.target.eeprom_caldata) release_firmware(fw_entry); fw_entry is currently assigned: if (ab->qmi.target.eeprom_caldata) { ... } else { snprintf(filename, sizeof(filename), "cal-%s-%s.bin", ath11k_bus_str(ab->hif.bus), dev_name(dev)); fw_entry = ath11k_core_firmware_request(ab, filename); } So unless I'm missing something it is always the case that fw_entry will be initialized when ab->qmi.target.eeprom_caldata is not set. > 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..9834e7dc5120 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 = {}; > struct fft_sample_ath11k *fft_sample = NULL; > u8 *data; > u32 data_len, i; so NAK on changes which "fix" false positives.
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..9834e7dc5120 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 = {}; 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/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 an 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, NULL, and {} initializers, respectively. 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 only. Also noticed by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- v2: use {} initializer (Jeff Johnson) and aggregate to the series --- drivers/net/wireless/ath/ath11k/qmi.c | 4 ++-- drivers/net/wireless/ath/ath11k/spectral.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)