0

Does this cause a memory leak because pWinsock didn't get deleted inside the fonction?

Winsock* CreateWinsock()
{
    Winsock* pWinsock=new Winsock;

    return pWinsock;
}

Edit: Actually, I cannot delete my pointer since it is a member of Game (pWinsock) that received the newly created Winsock in the code above. Is there anything wrong with this?

class Game{
public:
    Game();
    ~Game();

    void CreateWindowClass(HINSTANCE);
    void CreateRessources(HINSTANCE);

    void ShowLoginScreen();

    HWND Getm_hWnd();

public:
    D2DResources* pD2DResources;
    Winsock* pWinsock;
    MessageLog* pMessageLog;

private:
    HWND m_hWnd;
};
Mickael Bergeron Néron
  • 1,472
  • 1
  • 18
  • 31
  • 1
    As long as you delete it elsewhere. Really, a smart pointer helps. – chris Mar 17 '13 at 18:38
  • 1
    You **cannot** delete this in the function because then you would be returning a dangling pointer. – us2012 Mar 17 '13 at 18:38
  • What do you mean a smart pointer helps? – Mickael Bergeron Néron Mar 17 '13 at 18:38
  • @MickaelBergeronNéron, Because it frees the memory when you're done using it (a unique_ptr would free it when whatever you assign the result to goes out of scope), and unlike this, a smart pointer handles not assigning the result. For a reference on smart pointers, [see here](http://en.cppreference.com/w/cpp/memory). – chris Mar 17 '13 at 18:40
  • [Smart pointer](http://en.wikipedia.org/wiki/Smart_pointer), [std::shared_ptr](http://en.cppreference.com/w/cpp/memory/shared_ptr). Please, read it and come back if you have further questions. – Alexander Shukaev Mar 17 '13 at 18:40
  • 1
    If you can't delete the pointer then there most likely will be a memory leak. – David G Mar 18 '13 at 00:19
  • Many people told me the opposite. If you are right, then this would likely explain why I get tons of problem since I changed my code adding dynamic memory. – Mickael Bergeron Néron Mar 18 '13 at 00:26

3 Answers3

2

Watch out, if you delete the memory in the function the returned pointer will become a dangling pointer, as the memory for it has already been cleared. Dereferencing such a pointer is undefined behavior.

The program only causes a memory leak if the caller does not remember to delete the memory for themselves. Since you allocated memory within the function and returned it, it must be deleted somehow after the call. To delete the memory, it would look something like this:

Winsock *ptr = CreateWinsock(); // memory passed to ptr
// ...

delete ptr; // delete memory

The problem is that depending on the caller to delete the memory is quite cumbersome and unreliable. These kinds of potential problems can be alleviated through the use of smart pointers such as unique_ptr or shared_ptr. These objects delete the memory when their destructors are called, allowing great flexibility. Here's an example of how it would look for your program:

std::unique_ptr<Winsock> CreateWinsock()
{
    return std::unique_ptr<Winsock>(new Winsock);
}

std::unique_ptr<Winsock> ptr = CreateWinsock();

There's no need to explicitly delete the pointer as the encapsulating smart pointer now has that responsibility.

Community
  • 1
  • 1
David G
  • 94,763
  • 41
  • 167
  • 253
0

If the caller of this function deletes the pointer after it uses it, there is no leak. Therefore, it is not appropriate to comment on the memory leak given just this piece of code.

To refrain from such a case where the caller forgets to delete the object, use shared pointer, or another smart pointer.

meyumer
  • 5,063
  • 1
  • 17
  • 21
0

Nope, all that does is pass back the pointer to Winsock. For example

Winsock* ws = CreateWinsock();
ws->doSomething();
//......
//some more stuff
//......
//Finished using Winsock
delete ws;

If the delete wasn't called there when you're finished with Winsock then that would be seen as a memory leak because memory would be taken up by Winsock when its not being used anymore.

David Tr
  • 151
  • 2
  • 9
  • I cannot delete ws since it is a private member of Game. Is there anything wrong with this? – Mickael Bergeron Néron Mar 17 '13 at 18:55
  • No its not a private member, its a pointer to heap initialised memory and all you are doing is passing back the address of the memory. You cannot do this though `delete game.pWinsock;` because its a private member of the class Game. So all you do is create a Winsock and pass back the address through the method and this assigns it to a new pointer outside the class or wherever the method is being called, make sense? EDIT: Actually `delete game.pWinsock;` does work because its public in this example but assuming its private it does not work. – David Tr Mar 17 '13 at 19:04
  • So what would be the proper way to assign a pointer inside Game to the newly created object? – Mickael Bergeron Néron Mar 17 '13 at 19:06
  • 1
    What you have there is fine, it should work fine. Maybe you should make `pWinsock` private and in the method CreateWinsock have code like this. `Winsock* CreateWinsock() { pWinsock=new Winsock; return pWinsock; }`Then in the destructor you can destroy the Winsock when the class Game is destroyed or you can delete the ws like I did above, the choise is yours really. Just be careful because if you want to keep ws like above but want to destroy Game, then ws would be destroyed too if Game destroyed pWinSock in the destructor. – David Tr Mar 17 '13 at 19:10