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@xxxxxxxxx>
> ---
> 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.
