On 2015년 10월 04일 19:28, Chandra Gorentla wrote:
On Sun, Oct 04, 2015 at 09:44:57AM +0100, Greg KH wrote:
On Sat, Oct 03, 2015 at 02:57:30PM +0530, Chandra S Gorentla wrote:
- kfree is being called for the members of the queue without
de-queuing them; they are just inserted within this function;
they are supposed to be de-queued and freed in a function
for receiving the queue items
- goto statements are removed
- After kfree correction, there is no need for target block
of goto statement; hence it is removed
Signed-off-by: Chandra S Gorentla <csgorentla@xxxxxxxxx>
---
drivers/staging/wilc1000/wilc_msgqueue.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_msgqueue.c b/drivers/staging/wilc1000/wilc_msgqueue.c
index 284a3f5..eae90be 100644
--- a/drivers/staging/wilc1000/wilc_msgqueue.c
+++ b/drivers/staging/wilc1000/wilc_msgqueue.c
@@ -56,32 +56,30 @@ int wilc_mq_destroy(WILC_MsgQueueHandle *pHandle)
int wilc_mq_send(WILC_MsgQueueHandle *pHandle,
const void *pvSendBuffer, u32 u32SendBufferSize)
{
- int result = 0;
unsigned long flags;
Message *pstrMessage = NULL;
if ((!pHandle) || (u32SendBufferSize == 0) || (!pvSendBuffer)) {
PRINT_ER("pHandle or pvSendBuffer is null\n");
- result = -EFAULT;
- goto ERRORHANDLER;
+ return -EFAULT;
}
if (pHandle->bExiting) {
PRINT_ER("pHandle fail\n");
- result = -EFAULT;
- goto ERRORHANDLER;
+ return -EFAULT;
}
/* construct a new message */
pstrMessage = kmalloc(sizeof(Message), GFP_ATOMIC);
if (!pstrMessage)
return -ENOMEM;
+
pstrMessage->u32Length = u32SendBufferSize;
pstrMessage->pstrNext = NULL;
pstrMessage->pvBuffer = kmalloc(u32SendBufferSize, GFP_ATOMIC);
if (!pstrMessage->pvBuffer) {
- result = -ENOMEM;
- goto ERRORHANDLER;
+ kfree(pstrMessage);
+ return -ENOMEM;
}
memcpy(pstrMessage->pvBuffer, pvSendBuffer, u32SendBufferSize);
@@ -102,15 +100,7 @@ int wilc_mq_send(WILC_MsgQueueHandle *pHandle,
spin_unlock_irqrestore(&pHandle->strCriticalSection, flags);
up(&pHandle->hSem);
-
-ERRORHANDLER:
- /* error occured, free any allocations */
- if (pstrMessage) {
- kfree(pstrMessage->pvBuffer);
- kfree(pstrMessage);
- }
-
- return result;
+ return 0;
Aren't you now leaking memory as you aren't freeing pstrMessage and the
buffer on the "normal" return path?
In the normal path kfree is called in a separate (wilc_mq_recv) function.
The purpose of the currently modified function (wilc_mq_send) is to post
a message to a queue by allocating memory for the message. The receiver
function is supposed to remove the message from the queue and free the
memory.
This patch is reasonable and normal free is done in recv function as Chandra said.
Thanks,
Tony.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
