Official eMule-Board: Some Memory Leaks - Official eMule-Board

Jump to content


Page 1 of 1

Some Memory Leaks found by static code analyzer

#1 User is offline   gureedo 

  • Newbie
  • Pip
  • Group: Members
  • Posts: 10
  • Joined: 28-October 09

Posted 03 February 2010 - 11:35 AM

Sorry, but can't write here name of software i used for analyzing.

before:
CMMPacket* packet = new CMMPacket(MMP_PREVIEWANS);
	if (imgFrames != NULL && nCount != 0){
		packet->WriteByte(MMT_OK);
		CxImage* cur_frame = imgFrames[0];
		if (cur_frame == NULL){
			ASSERT ( false );
			return;
		}
		BYTE* abyResultBuffer = NULL;
		long nResultSize = 0;
		if (!cur_frame->Encode(abyResultBuffer, nResultSize, CXIMAGE_FORMAT_PNG)){
			ASSERT ( false );
			return;
		}
		packet->WriteInt(nResultSize);
		packet->m_pBuffer->Write(abyResultBuffer, nResultSize);
		free(abyResultBuffer);
	}


after:
CMMPacket* packet = new CMMPacket(MMP_PREVIEWANS);
	if (imgFrames != NULL && nCount != 0){
		packet->WriteByte(MMT_OK);
		CxImage* cur_frame = imgFrames[0];
		if (cur_frame == NULL){
			ASSERT ( false );
			delete packet; // FIXED HERE!!!
			return;
		}
		BYTE* abyResultBuffer = NULL;
		long nResultSize = 0;
		if (!cur_frame->Encode(abyResultBuffer, nResultSize, CXIMAGE_FORMAT_PNG)){
			ASSERT ( false );
			delete packet; // FIXED HERE!!!
			return;
		}
		packet->WriteInt(nResultSize);
		packet->m_pBuffer->Write(abyResultBuffer, nResultSize);
		free(abyResultBuffer);
	}

This post has been edited by gureedo: 04 February 2010 - 05:14 AM

0

#2 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Posted 03 February 2010 - 12:30 PM

Good eye! Though I want to suggest a different approach:

	if (m_pPendingCommandSocket == NULL)
		return;

//>>> WiZaRd
	bool bSucc = false;
	CMMPacket* packet = new CMMPacket(MMP_PREVIEWANS);
//<<< WiZaRd	
	if (imgFrames != NULL && nCount != 0)
	{
//>>> WiZaRd
//		packet->WriteByte(MMT_OK);
//<<< WiZaRd
		CxImage* cur_frame = imgFrames[0];
		if (cur_frame == NULL)
		{
			ASSERT(0);
//>>> WiZaRd
//			delete packet; //>>> Memleak FiX
//			return;
			break;
//<<< WiZaRd
		}
		BYTE* abyResultBuffer = NULL;
		long nResultSize = 0;
		if (!cur_frame->Encode(abyResultBuffer, nResultSize, CXIMAGE_FORMAT_PNG))
		{
			ASSERT(0);
//>>> WiZaRd
//			delete packet; //>>> Memleak FiX
//			return;
			break;
//<<< WiZaRd
		}
//>>> WiZaRd
		bSucc = true;
		packet->WriteByte(MMT_OK);
//<<< WiZaRd
		packet->WriteInt(nResultSize);
		packet->m_pBuffer->Write(abyResultBuffer, nResultSize);
		free(abyResultBuffer);
	}
//>>> WiZaRd
//	else
	if(!bSucc)
//<<< WiZaRd
		packet->WriteByte(MMT_FAILED);

	m_pPendingCommandSocket->SendPacket(packet);
	m_pPendingCommandSocket = NULL;


That way, we will always send an answer, so the MM knows if we fail to read the image.


EDiT: btw: the same goes for CUpDownClient::SendPreviewAnswer

This post has been edited by tHeWiZaRdOfDoS: 03 February 2010 - 12:31 PM

0

#3 User is offline   gureedo 

  • Newbie
  • Pip
  • Group: Members
  • Posts: 10
  • Joined: 28-October 09

Posted 03 February 2010 - 12:56 PM

it's not my eye, it's static code analyzer.
0

#4 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Posted 03 February 2010 - 01:04 PM

Neat... mind to tell us what software you're using?
0

#5 User is offline   gureedo 

  • Newbie
  • Pip
  • Group: Members
  • Posts: 10
  • Joined: 28-October 09

Posted 04 February 2010 - 05:17 AM

at archiverecovery.cpp:
method bool CArchiveRecovery::recoverAce(...)
before:
while ((block = scanForAceFileHeader(aceInput, aitp, (fill->end - filesearchstart ))) != NULL)
{
	if (aitp){
		if (!aitp->m_bIsValid)
			return false;
	}

after:
while ((block = scanForAceFileHeader(aceInput, aitp, (fill->end - filesearchstart ))) != NULL)
{
	if (aitp){
		if (!aitp->m_bIsValid){
			delete block;
			return false;
		}
	}

0

#6 User is offline   taz-me 

  • I'm taz (a modder)
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 587
  • Joined: 07-December 06

Posted 07 February 2010 - 11:25 AM

Since CMMServer::PreviewFinished does not contain loops - I guess "break" should be replaced by "BAD" goto or using additional "else"s.

As for the static code analyzer tool : from some checks and googling it seems there's only one "real" tool that might provide this type of outcome ...
P2P is about sharing, ed2k is my choice !
0

#7 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Posted 07 February 2010 - 12:03 PM

^^ break should be correct as you would leave the
if (imgFrames != NULL && nCount != 0)
block and continue with
if(!bSucc)
0

#8 User is offline   taz-me 

  • I'm taz (a modder)
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 587
  • Joined: 07-December 06

Posted 07 February 2010 - 12:54 PM

logically the code was fine :+1: , however VS2008 complained with something like (I can't recall exact compiler error) : "invalid break statement" ...
P2P is about sharing, ed2k is my choice !
0

#9 User is offline   p!ll3.p4ll0 

  • Splendid Member
  • PipPipPipPip
  • Group: Members
  • Posts: 124
  • Joined: 29-December 02

Posted 21 February 2010 - 05:59 PM

V$ 2k3 doesn't like it either.

Compiler Error C2043 - illegal break - A break is legal only within a do, for, while, or switch statement ;)

Else'ing it up would end with something like
	bool bSucc = false;
	CMMPacket* packet = new CMMPacket(MMP_PREVIEWANS);   
	if (imgFrames != NULL && nCount != 0)
	{
		CxImage* cur_frame = imgFrames[0];
		if (cur_frame == NULL)
		{
			ASSERT(0);
		}
		else
		{
			BYTE* abyResultBuffer = NULL;
			long nResultSize = 0;
			if (!cur_frame->Encode(abyResultBuffer, nResultSize, CXIMAGE_FORMAT_PNG))
			{
				ASSERT(0);
			}
			else
			{
				bSucc = true;
				packet->WriteByte(MMT_OK);
				packet->WriteInt(nResultSize);
				packet->m_pBuffer->Write(abyResultBuffer, nResultSize);
				free(abyResultBuffer);
			}
		}
	}

	if(!bSucc)
		packet->WriteByte(MMT_FAILED);

0

#10 User is offline   taz-me 

  • I'm taz (a modder)
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 587
  • Joined: 07-December 06

Posted 21 February 2010 - 06:22 PM

View Postp!ll3.p4ll0, on 21 February 2010 - 07:59 PM, said:

V$ 2k3 doesn't like it either.

Compiler Error C2043 - illegal break - A break is legal only within a do, for, while, or switch statement ;)


If you have just looked at post no. 6 here :P
P2P is about sharing, ed2k is my choice !
0

#11 User is offline   p!ll3.p4ll0 

  • Splendid Member
  • PipPipPipPip
  • Group: Members
  • Posts: 124
  • Joined: 29-December 02

Posted 21 February 2010 - 07:09 PM

I never read further than the fifth posting - for longer threads I always wait till they come out as a movie.

Seriously, I needed my annual posting here, and the sofar unfixed fix code was a good opportunity.
0

  • Member Options

Page 1 of 1

1 User(s) are reading this topic
0 members, 1 guests, 0 anonymous users