Honorable Member Mali 42182 Posted March 1, 2020 Honorable Member Share Posted March 1, 2020 (edited) 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 1 1 24 Link to comment Share on other sites More sharing options...
Hik 108 Posted March 1, 2020 Share Posted March 1, 2020 Thanks for what you do 1 Link to comment Share on other sites More sharing options...
SafariXD 8 Posted March 1, 2020 Share Posted March 1, 2020 Such a good developer! Link to comment Share on other sites More sharing options...
OtherChoice 77 Posted March 2, 2020 Share Posted March 2, 2020 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; } 1 Link to comment Share on other sites More sharing options...
Honorable Member Mali 42182 Posted March 2, 2020 Author Honorable Member Share Posted March 2, 2020 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 Link to comment Share on other sites More sharing options...
Active Member hachiwari 138 Posted March 2, 2020 Active Member Share Posted March 2, 2020 13 hours ago, Mali61 said: Actually I wouldn't prefer this method Why? Link to comment Share on other sites More sharing options...
Premium masodikbela 1362 Posted March 3, 2020 Premium Share Posted March 3, 2020 (edited) The second one is totally wrong and causes double deletion. Edited August 25, 2022 by Metin2 Dev Core X - External 2 Internal 3 The one and only UI programming guideline Link to comment Share on other sites More sharing options...
Honorable Member Mali 42182 Posted March 3, 2020 Author Honorable Member Share Posted March 3, 2020 (edited) 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 1 Link to comment Share on other sites More sharing options...
Active+ Member enisina 172 Posted March 6, 2020 Active+ Member Share Posted March 6, 2020 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. 1 Link to comment Share on other sites More sharing options...
OtherChoice 77 Posted March 9, 2020 Share Posted March 9, 2020 @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. Link to comment Share on other sites More sharing options...
Alpha 484 Posted March 13, 2020 Share Posted March 13, 2020 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 Link to comment Share on other sites More sharing options...
Mafuyu 51 Posted March 14, 2020 Share Posted March 14, 2020 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? Link to comment Share on other sites More sharing options...
Honorable Member Mali 42182 Posted March 16, 2020 Author Honorable Member Share Posted March 16, 2020 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. 1 Link to comment Share on other sites More sharing options...
Recommended Posts