Fix possible memory leak

[haraldreif]

In MQTTAsync_receiveThread, when pack is not NULL but none of the if-else if-conditions is fulfilled, pack is not freed. To fix this, an else block has been added to cleanup the memory of pack.

Signed-off-by: Harald Reif [email protected]

[JuergenKosel]

Hello,

I have integrated the commit of this pull request into my application. Because I wanted to verify if this fixes my memory leak found by valgrind :

==17== 13,380 (48 direct, 13,332 indirect) bytes in 1 blocks are definitely lost in loss record 143 of 158
==17==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==17==    by 0x1B8E2601: TreeAddByIndex (Tree.c:248)
==17==    by 0x1B8E279B: TreeAdd (Tree.c:280)
==17==    by 0x1B8FA5F9: mymalloc (Heap.c:213)
==17==    by 0x1B8D0AC8: Protocol_processPublication (MQTTAsyncUtils.c:2586)
==17==    by 0x1B8D8482: MQTTProtocol_handlePublishes (MQTTProtocolClient.c:339)
==17==    by 0x1B8D2D0B: MQTTAsync_cycle (MQTTAsyncUtils.c:3060)
==17==    by 0x1B8CE2DB: MQTTAsync_receiveThread (MQTTAsyncUtils.c:2032)
==17==    by 0x8CD6EA6: start_thread (pthread_create.c:477)
==17==    by 0x9458A2E: clone (clone.S:95)
==17== 
==17== error_end
==17== error_begin
==17== 46,399 (48 direct, 46,351 indirect) bytes in 1 blocks are definitely lost in loss record 155 of 158
==17==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==17==    by 0x1B8E2601: TreeAddByIndex (Tree.c:248)
==17==    by 0x1B8E279B: TreeAdd (Tree.c:280)
==17==    by 0x1B8FA5F9: mymalloc (Heap.c:213)
==17==    by 0x1B8DDE80: MQTTPacket_publish (MQTTPacket.c:563)
==17==    by 0x1B8DCB87: MQTTPacket_Factory (MQTTPacket.c:148)
==17==    by 0x1B8D2BFA: MQTTAsync_cycle (MQTTAsyncUtils.c:3047)
==17==    by 0x1B8CE2DB: MQTTAsync_receiveThread (MQTTAsyncUtils.c:2032)
==17==    by 0x8CD6EA6: start_thread (pthread_create.c:477)
==17==    by 0x9458A2E: clone (clone.S:95)
==17== 
==17== error_end

Unfortunately the memory leak is still there!

Valgrind reports also this memory leaks in the context of the paho.mqtt.c library.
So I will create an issue for these memory leaks.

[JuergenKosel]

I believe, that is not sufficient.
Because pack points to many different structures.
E.g. if allocated inside MQTTPacket_publish(int MQTTVersion, unsigned char aHeader, char* data, size_t datalen)

pack points to this structure:

/**
 * Data for a publish packet.
 */
typedef struct
{
	Header header;	/**< MQTT header byte */
	char* topic;	/**< topic string */
	int topiclen;
	int msgId;		/**< MQTT message id */
	char* payload;	/**< binary payload, length delimited */
	int payloadlen;	/**< payload length */
	int MQTTVersion;  /**< the version of MQTT */
	MQTTProperties properties; /**< MQTT 5.0 properties.  Not used for MQTT < 5.0 */
	uint8_t mask[4]; /**< the websockets mask the payload is masked with, if any */
} Publish;

Which includes pointers to other allocated memory, which is not release in this case.
See issue #1518

[icraggs]

I would be very surprised if this fixed any memory leak under usual circumstances, because this is a "should never happen" case. I think it's impossible to reach, and the correct action, if there is one, is probably just to write an error message to the log saying that this situation had occurred.

[icraggs]

I don't think this is a useful addition - just add a log message.