Re: [PATCH 5/5] ath10k: pull_svc_rdy code-style fix

[Tom Psyborg <pozega.tomislav@gmail.com>]

On 24/09/2019, Kalle Valo <kvalo@codeaurora.org> wrote:
> Tomislav Požega <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žega <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 ath10k
>> *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 = (void *)skb->data;
>>  	size_t i, n;
>>
>>  	if (skb->len < sizeof(*ev))
>>  		return -EPROTO;
>>
>> -	ev = (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.