From: Jeff Johnson <quic_jjohnson@quicinc.com>
To: Dmitry Antipov <dmantipov@yandex.ru>
Cc: Baochen Qiang <quic_bqiang@quicinc.com>,
Kalle Valo <kvalo@kernel.org>, <ath11k@lists.infradead.org>,
<linux-wireless@vger.kernel.org>, <lvc-project@linuxtesting.org>
Subject: Re: [PATCH 3/3] [v2] wifi: ath11k: fix few -Wmaybe-uninitialized warnings
Date: Thu, 29 Feb 2024 13:00:14 -0800 [thread overview]
Message-ID: <8d0fd9bc-6da1-4d72-962a-60074940e1bb@quicinc.com> (raw)
In-Reply-To: <20240229084031.51957-3-dmantipov@yandex.ru>
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.
next prev parent reply other threads:[~2024-02-29 21:00 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-28 13:14 [PATCH] wifi: ath11k: fix few -Wmaybe-uninitialized warnings Dmitry Antipov
2024-02-28 14:01 ` Kalle Valo
2024-02-28 14:13 ` Dmitry Antipov
2024-02-28 15:54 ` Jeff Johnson
2024-02-29 8:40 ` [PATCH 1/3] [v2] wifi: ath11k: use ath11k_mac_get_ar_by_pdev_id() consistently Dmitry Antipov
2024-02-29 8:40 ` [PATCH 2/3] [v2] wifi: ath11k: handle unknown scan state in ath11k_mac_op_remain_on_channel() Dmitry Antipov
2024-02-29 20:18 ` Jeff Johnson
2024-02-29 8:40 ` [PATCH 3/3] [v2] wifi: ath11k: fix few -Wmaybe-uninitialized warnings Dmitry Antipov
2024-02-29 21:00 ` Jeff Johnson [this message]
2024-02-29 20:10 ` [PATCH 1/3] [v2] wifi: ath11k: use ath11k_mac_get_ar_by_pdev_id() consistently Jeff Johnson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8d0fd9bc-6da1-4d72-962a-60074940e1bb@quicinc.com \
--to=quic_jjohnson@quicinc.com \
--cc=ath11k@lists.infradead.org \
--cc=dmantipov@yandex.ru \
--cc=kvalo@kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lvc-project@linuxtesting.org \
--cc=quic_bqiang@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).