Hi Dan,
On Tue, 3 Jul 2018 14:56:55 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> Hello Ajay Singh,
>
> The patch ff52a57a7a42: "staging: wilc1000: move the allocation of
> cmd out of wilc_enqueue_cmd()" from Jun 26, 2018, leads to the
> following static checker warning:
>
> drivers/staging/wilc1000/host_interface.c:3390 wilc_deinit()
> warn: inconsistent returns 'hif_deinit_lock'.
Thanks for reporting the static check warning.
>
> drivers/staging/wilc1000/host_interface.c
> 3348 mutex_lock(&hif_deinit_lock);
> 3349
> 3350 terminated_handle = hif_drv;
> 3351
> 3352 del_timer_sync(&hif_drv->scan_timer);
> 3353 del_timer_sync(&hif_drv->connect_timer);
> 3354 del_timer_sync(&periodic_rssi);
> 3355 del_timer_sync(&hif_drv->remain_on_ch_timer);
> 3356
> 3357 wilc_set_wfi_drv_handler(vif, 0, 0, 0);
> 3358 wait_for_completion(&hif_driver_comp);
> 3359
> 3360 if (hif_drv->usr_scan_req.scan_result) {
> 3361
> hif_drv->usr_scan_req.scan_result(SCAN_EVENT_ABORTED, NULL,
> 3362
> hif_drv->usr_scan_req.arg,
> 3363 NULL);
> 3364 hif_drv->usr_scan_req.scan_result = NULL;
> 3365 } 3366 3367 hif_drv->hif_state = HOST_IF_IDLE;
> 3368
> 3369 if (clients_count == 1) {
> 3370 struct host_if_msg *msg;
> 3371
> 3372 msg = wilc_alloc_work(vif,
> handle_hif_exit_work, true); 3373 if (IS_ERR(msg))
> 3374 return PTR_ERR(msg);
> ^^^^^^^^^^^^
> We should unlock. Probably set terminated_handle to NULL as well?
>
Sure, I will add the code to unlock and set the terminated_handle to
NULL and cleanup other variables.
As its error path during the cleanup flow, so i think we should
only avoid the posting of work_queue and rest of the cleanup code should
be executed to cleanup.
Something like below i.e instead of using direct return only block
code which depends on 'msg' under if condition and execute the rest of
cleanup code for all scenario.
if (!IS_ERR(msg)) {
result = wilc_enqueue_work(msg);
if (result)
netdev_err(vif->ndev, "deinit : Error(%d)\n",result);
else
wait_for_completion(&msg->work_comp);
kfree(msg);
}
destroy_workqueue(hif_workqueue);
I will include these changes and submit the patch.
> 3375
> 3376 result = wilc_enqueue_work(msg);
> 3377 if (result)
> 3378 netdev_err(vif->ndev, "deinit :
> Error(%d)\n", result); 3379 else
> 3380 wait_for_completion(&msg->work_comp);
> 3381 kfree(msg);
> 3382 destroy_workqueue(hif_workqueue);
> 3383 }
> 3384
> 3385 kfree(hif_drv);
> 3386
> 3387 clients_count--;
> 3388 terminated_handle = NULL;
> 3389 mutex_unlock(&hif_deinit_lock);
> 3390 return result;
> 3391 }
>
> regards,
> dan carpenter
> _______________________________________________
> devel mailing list
> devel@xxxxxxxxxxxxxxxxxxxxxx
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
