| Message ID | 20200619142922.GA267142@mwanda |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] ath11k: fix uninitialized return in ath11k_spectral_process_data() | expand |
Dan Carpenter <dan.carpenter@oracle.com> writes: > On Mon, Jun 22, 2020 at 05:51:16PM +0300, Kalle Valo wrote: >> Dan Carpenter <dan.carpenter@oracle.com> writes: >> >> > There is a success path where "ret" isn't initialized where we never >> > have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized. >> > >> > Fixes: 9d11b7bff950 ("ath11k: add support for spectral scan") >> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> > --- >> > drivers/net/wireless/ath/ath11k/spectral.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c >> > index 1c5d65bb411f..bfbf905f7507 100644 >> > --- a/drivers/net/wireless/ath/ath11k/spectral.c >> > +++ b/drivers/net/wireless/ath/ath11k/spectral.c >> > @@ -677,7 +677,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar, >> > u32 data_len, i; >> > u8 sign, tag; >> > int tlv_len, sample_sz; >> > - int ret; >> > + int ret = 0; >> > bool quit = false; >> >> I try to avoid initialising ret variables so I would like find another >> way. What about doing this (completely untested!) in the end of the >> function: >> >> return 0; >> >> err: >> kfree(fft_sample); >> unlock: >> spin_unlock_bh(&ar->spectral.lock); >> return ret; > > I normally avoid it as well... If I were to redo this patch, I would > probably do: > > ret = 0; > err: > kfree(fft_sample); > unlock: > spin_unlock_bh(&ar->spectral.lock); > return ret; > > Would that be better? Ah, my proposal was buggy. So yes, this is definitely better :) I changed now the patch in the pending branch, please check: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=586f8e6779fa75ade112cb48606794e034e0ff1d -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Dan Carpenter <dan.carpenter@oracle.com> wrote: > There is a success path where "ret" isn't initialized where we never > have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized. > > Fixes: 9d11b7bff950 ("ath11k: add support for spectral scan") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. c7187acc3cd0 ath11k: fix uninitialized return in ath11k_spectral_process_data() -- https://patchwork.kernel.org/patch/11614313/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c index 1c5d65bb411f..bfbf905f7507 100644 --- a/drivers/net/wireless/ath/ath11k/spectral.c +++ b/drivers/net/wireless/ath/ath11k/spectral.c @@ -677,7 +677,7 @@ static int ath11k_spectral_process_data(struct ath11k *ar, u32 data_len, i; u8 sign, tag; int tlv_len, sample_sz; - int ret; + int ret = 0; bool quit = false; spin_lock_bh(&ar->spectral.lock);
There is a success path where "ret" isn't initialized where we never have a ATH11K_SPECTRAL_TAG_SCAN_SEARCH and then ret isn't initialized. Fixes: 9d11b7bff950 ("ath11k: add support for spectral scan") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/net/wireless/ath/ath11k/spectral.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)