Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Ramon Fried]

On 17 May 2018 at 21:37, Jeff Johnson  wrote:
> On 2018-05-17 04:32, Ramon Fried wrote:
>>
>> From: Eyal Ilsar 
>
> ...
>>
>> +int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
>> +   struct ieee80211_vif *vif, void *ptt_msg,
>> size_t len,
>> +   void **ptt_rsp_msg)
>> +{
>> +   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
>> +   int ret = 0;
>> +
>> +   mutex_lock(>hal_mutex);
>> +   p_msg_body = kmalloc(
>> +   sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
>> +   GFP_ATOMIC);
>
>
> NULL check required?
>
>> +   INIT_HAL_PTT_MSG(p_msg_body, len);
>> +

Thanks Jeff. will fix it and send again :)

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Ramon Fried]

On 17 May 2018 at 21:37, Jeff Johnson  wrote:
> On 2018-05-17 04:32, Ramon Fried wrote:
>>
>> From: Eyal Ilsar 
>
> ...
>>
>> +int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
>> +   struct ieee80211_vif *vif, void *ptt_msg,
>> size_t len,
>> +   void **ptt_rsp_msg)
>> +{
>> +   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
>> +   int ret = 0;
>> +
>> +   mutex_lock(>hal_mutex);
>> +   p_msg_body = kmalloc(
>> +   sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
>> +   GFP_ATOMIC);
>
>
> NULL check required?
>
>> +   INIT_HAL_PTT_MSG(p_msg_body, len);
>> +

Thanks Jeff. will fix it and send again :)

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;
+
+   mutex_lock(>hal_mutex);
+   p_msg_body = kmalloc(
+   sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
+   GFP_ATOMIC);


NULL check required?


+   INIT_HAL_PTT_MSG(p_msg_body, len);
+

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;
+
+   mutex_lock(>hal_mutex);
+   p_msg_body = kmalloc(
+   sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
+   GFP_ATOMIC);


NULL check required?


+   INIT_HAL_PTT_MSG(p_msg_body, len);
+

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+static int wcn36xx_smd_process_ptt_msg_rsp(void *buf, size_t len,
+  void **p_ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_rsp_msg *rsp;
+   int ret = 0;


why initialize 'ret' when you immediately overwrite?


+   ret = wcn36xx_smd_rsp_status_check(buf, len);

...

+   if (rsp->header.len > 0) {
+   *p_ptt_rsp_msg = kmalloc(rsp->header.len, GFP_ATOMIC);


NULL check required?


+   memcpy(*p_ptt_rsp_msg, rsp->ptt_msg, rsp->header.len);
+   }
+   return ret;
+}
+
+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;


why initialize 'ret' when it is always overwritten before use?


+   ret = wcn36xx_smd_send_and_wait(wcn, p_msg_body->header.len);

Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)

[Jeff Johnson]

On 2018-05-17 04:32, Ramon Fried wrote:

From: Eyal Ilsar 

...

+static int wcn36xx_smd_process_ptt_msg_rsp(void *buf, size_t len,
+  void **p_ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_rsp_msg *rsp;
+   int ret = 0;


why initialize 'ret' when you immediately overwrite?


+   ret = wcn36xx_smd_rsp_status_check(buf, len);

...

+   if (rsp->header.len > 0) {
+   *p_ptt_rsp_msg = kmalloc(rsp->header.len, GFP_ATOMIC);


NULL check required?


+   memcpy(*p_ptt_rsp_msg, rsp->ptt_msg, rsp->header.len);
+   }
+   return ret;
+}
+
+int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
+   struct ieee80211_vif *vif, void *ptt_msg, 
size_t len,
+   void **ptt_rsp_msg)
+{
+   struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
+   int ret = 0;


why initialize 'ret' when it is always overwritten before use?


+   ret = wcn36xx_smd_send_and_wait(wcn, p_msg_body->header.len);