Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <87d0fq5kic.fsf@codeaurora.org>
References: <1569268165-1639-1-git-send-email-pozega.tomislav@gmail.com>
 <1569268165-1639-5-git-send-email-pozega.tomislav@gmail.com> <87d0fq5kic.fsf@codeaurora.org>
From:   Tom Psyborg <pozega.tomislav@gmail.com>
Date:   Tue, 24 Sep 2019 09:49:18 +0200
Message-ID: <CAKR_QVK=XwLtaGgoLeU5-+XQP_-jVvAdWfkGvdyV9WNK_5QUng@mail.gmail.com>
Subject: Re: [PATCH 5/5] ath10k: pull_svc_rdy code-style fix
To:     Kalle Valo <kvalo@codeaurora.org>
Cc:     linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 24/09/2019, Kalle Valo <kvalo@codeaurora.org> wrote:
> Tomislav Po=C5=BEega <pozega.tomislav@gmail.com> writes:
>
>> Drop unneeded lines by moving skb data in both main and 10x WMI
>> pull service ready event operations.
>>
>> Signed-off-by: Tomislav Po=C5=BEega <pozega.tomislav@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/wmi.c |    6 ++----
>>  1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
>> b/drivers/net/wireless/ath/ath10k/wmi.c
>> index 59d2d2a..8ab178c 100644
>> --- a/drivers/net/wireless/ath/ath10k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
>> @@ -5345,13 +5345,12 @@ static int ath10k_wmi_alloc_host_mem(struct ath1=
0k
>> *ar, u32 req_id,
>>  ath10k_wmi_main_op_pull_svc_rdy_ev(struct ath10k *ar, struct sk_buff
>> *skb,
>>  				   struct wmi_svc_rdy_ev_arg *arg)
>>  {
>> -	struct wmi_service_ready_event *ev;
>> +	struct wmi_service_ready_event *ev =3D (void *)skb->data;
>>  	size_t i, n;
>>
>>  	if (skb->len < sizeof(*ev))
>>  		return -EPROTO;
>>
>> -	ev =3D (void *)skb->data;
>
> Actually I prefer the original style, so that we first check the data in
> skb is valid and only then assign the data to ev.
>
> --
> Kalle Valo
>

It came to my mind that this might be the reason why the current
driver did not give me warning about too short service ready event,
but there was no warning about event length in either case.
I even tested this with compat wireless from 2013. and there the
situation was the opposite: in both cases there was warning about
service ready length.