fix Safebox Memory Leak Fix

[Mali 61380]

Find in ClientManager.cpp(x2):

				pkPeer->EncodeHeader(HEADER_DG_SAFEBOX_WRONG_PASSWORD, dwHandle, 0);

Add(x2):

				delete pSafebox;

 

 

safebox.cpp Find:

	if (pkOldGrid)
		m_pkGrid = M2_NEW CGrid(pkOldGrid, 5, m_iSize);
	else
		m_pkGrid = M2_NEW CGrid(5, m_iSize);

Change:

	if (pkOldGrid) {
		m_pkGrid = M2_NEW CGrid(pkOldGrid, 5, m_iSize);
		delete pkOldGrid;
	}
	else
		m_pkGrid = M2_NEW CGrid(5, m_iSize);

 

 

 

Edited August 25, 2022 by Metin2 Dev
Core X - External 2 Internal

[Hik 124]

Thanks for what you do

[SafariXD 11]

Such a good developer!

[OtherChoice 81]

Nice shot. For better code management and reusability you could remove the delete from the if (pkOldGrid) check and put it into the override method CGrid::CGrid(CGrid * pkGrid, int w, int h) : m_iWidth(w), m_iHeight(h) like this:
 

CGrid::CGrid(int w, int h) : m_iWidth(w), m_iHeight(h)					
{
	m_pGrid = new char[m_iWidth * m_iHeight];						
	memset(m_pGrid, 0, sizeof(char) * m_iWidth * m_iHeight);
}

CGrid::CGrid(CGrid * pkGrid, int w, int h) : m_iWidth(w), m_iHeight(h)		//<- if you add delete pkGrid here you can call M2_NEW CGrid(CGrid*, width, height) and it will delete the old pointer of safebox function at the end of this method 
{
	m_pGrid = new char[m_iWidth * m_iHeight];
	int iSize = std::MIN(w * h, pkGrid->m_iWidth * pkGrid->m_iHeight);
	thecore_memcpy(m_pGrid, pkGrid->m_pGrid, sizeof(char) * iSize);
  	//here
	delete pkGrid;
}

 

[Mali 61380]

49 minutes ago, OtherChoice said:

Nice shot. For better code management and reusability you could remove the delete from the if (pkOldGrid) check and put it into the override method CGrid::CGrid(CGrid * pkGrid, int w, int h) : m_iWidth(w), m_iHeight(h) like this:
 

CGrid::CGrid(int w, int h) : m_iWidth(w), m_iHeight(h)					
{
	m_pGrid = new char[m_iWidth * m_iHeight];						
	memset(m_pGrid, 0, sizeof(char) * m_iWidth * m_iHeight);
}

CGrid::CGrid(CGrid * pkGrid, int w, int h) : m_iWidth(w), m_iHeight(h)		//<- if you add delete pkGrid here you can call M2_NEW CGrid(CGrid*, width, height) and it will delete the old pointer of safebox function at the end of this method 
{
	m_pGrid = new char[m_iWidth * m_iHeight];
	int iSize = std::MIN(w * h, pkGrid->m_iWidth * pkGrid->m_iHeight);
	thecore_memcpy(m_pGrid, pkGrid->m_pGrid, sizeof(char) * iSize);
  	//here
	delete pkGrid;
}

 

Actually I wouldn't prefer this method

[hachiwari 144]

13 hours ago, Mali61 said:

Actually I wouldn't prefer this method

Why?

[masodikbela 1751]

The second one is totally wrong and causes double deletion.

Edited August 25, 2022 by Metin2 Dev
Core X - External 2 Internal

[Mali 61380]

29 minutes ago, masodikbela said:

The second one is totally wrong and causes double deletion.

I didn't see thanks

Edited August 25, 2022 by Metin2 Dev
Core X - External 2 Internal

[enisina 242]

On 3/2/2020 at 8:44 AM, OtherChoice said:

Nice shot. For better code management and reusability you could remove the delete from the if (pkOldGrid) check and put it into the override method CGrid::CGrid(CGrid * pkGrid, int w, int h) : m_iWidth(w), m_iHeight(h) like this:
 

CGrid::CGrid(int w, int h) : m_iWidth(w), m_iHeight(h)					
{
	m_pGrid = new char[m_iWidth * m_iHeight];						
	memset(m_pGrid, 0, sizeof(char) * m_iWidth * m_iHeight);
}

CGrid::CGrid(CGrid * pkGrid, int w, int h) : m_iWidth(w), m_iHeight(h)		//<- if you add delete pkGrid here you can call M2_NEW CGrid(CGrid*, width, height) and it will delete the old pointer of safebox function at the end of this method 
{
	m_pGrid = new char[m_iWidth * m_iHeight];
	int iSize = std::MIN(w * h, pkGrid->m_iWidth * pkGrid->m_iHeight);
	thecore_memcpy(m_pGrid, pkGrid->m_pGrid, sizeof(char) * iSize);
  	//here
	delete pkGrid;
}

 

you are just doing excess code.

[OtherChoice 81]

@enisina Maybe I didn't explain my thoughts correctly, sorry. What I meant is to add delete pkGrid; to the already existing method override . No excess code, it is already in your source.

[Alpha 757]

Deleting the old grid in the constructor of the new grid just makes destruction less obvious and when working with raw pointers to heap memory you do not want destruction to be hidden.

As it can easily lead to heap use after free bugs

[Mafuyu 56]

On 3/3/2020 at 10:54 AM, Mali61 said:

I didn't see thanks

u already edited it? is the guide now correct or is the second fix still the double deletion?

[Mali 61380]

On 3/15/2020 at 1:53 AM, Mafuyu said:

u already edited it? is the guide now correct or is the second fix still the double deletion?

correct...

I have already deleted the wrong one.