On Mon, Oct 19, 2015 at 06:03:15PM +0900, Tony Cho wrote:
> From: Leo Kim <leo.kim@xxxxxxxxx>
>
> This patch uses goto statement to separet error conditionals into release_lock
> and mem_free in wilc_mq_send. If unexpected errors happen, this function
> cannot up the semaphore, otherwise, if no errors, the semaphore should be
> released, but freeing memory is not needed in this function.
>
> Signed-off-by: Leo Kim <leo.kim@xxxxxxxxx>
> Signed-off-by: Tony Cho <tony.cho@xxxxxxxxx>
> ---
> V3: use goto statement to seperate error types into release_lock and mem_free
> ---
> drivers/staging/wilc1000/wilc_msgqueue.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_msgqueue.c b/drivers/staging/wilc1000/wilc_msgqueue.c
> index b13809a..bb4e93c 100644
> --- a/drivers/staging/wilc1000/wilc_msgqueue.c
> +++ b/drivers/staging/wilc1000/wilc_msgqueue.c
> @@ -63,28 +63,31 @@ int wilc_mq_send(WILC_MsgQueueHandle *pHandle,
> if ((!pHandle) || (u32SendBufferSize == 0) || (!pvSendBuffer)) {
> PRINT_ER("pHandle or pvSendBuffer is null\n");
> result = -EFAULT;
> - goto ERRORHANDLER;
> + goto out;
Here you can return -EFAULT beacuse no cleanup is required
> }
>
> if (pHandle->bExiting) {
> PRINT_ER("pHandle fail\n");
> result = -EFAULT;
> - goto ERRORHANDLER;
> + goto out;
> }
Ditto
> spin_lock_irqsave(&pHandle->strCriticalSection, flags);
>
> /* construct a new message */
> pstrMessage = kmalloc(sizeof(Message), GFP_ATOMIC);
> - if (!pstrMessage)
> - return -ENOMEM;
> + if (!pstrMessage) {
> + result = -ENOMEM;
> + goto release_lock;
> + }
> +
You can move the allocation outside the lock, then if the allocation
fails you can directly return -ENOMEM.
> pstrMessage->u32Length = u32SendBufferSize;
> pstrMessage->pstrNext = NULL;
> pstrMessage->pvBuffer = kmemdup(pvSendBuffer, u32SendBufferSize,
> GFP_ATOMIC);
> if (!pstrMessage->pvBuffer) {
> result = -ENOMEM;
> - goto ERRORHANDLER;
> + goto mem_free;
> }
>
> /* add it to the message queue */
> @@ -103,13 +106,13 @@ int wilc_mq_send(WILC_MsgQueueHandle *pHandle,
>
> up(&pHandle->hSem);
>
> -ERRORHANDLER:
> - /* error occured, free any allocations */
> - if (pstrMessage) {
> - kfree(pstrMessage->pvBuffer);
> - kfree(pstrMessage);
> - }
> + return 0;
>
> +mem_free:
> + kfree(pstrMessage);
> +release_lock:
> + spin_unlock_irqrestore(&pHandle->strCriticalSection, flags);
> +out:
> return result;
> }
>
> --
> 1.9.1
--
Sincerely yours,
Mike.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
