-4

e.g.

#include <iostream>

class JMP
{
    public:
        std::string number;
        JMP() {}
        JMP(const char* num) { number = num; }
        JMP operator+(const JMP &j)
        {
            // Allocate object
            JMP *sum_obj = new JMP("0");
            sum_obj->number[0] += 1;
            return *sum_obj;
        } // <--- memory leak
};

int main()
{
    JMP *a = new JMP(), *b = new JMP(), *c = new JMP();
    *c = *a + *b;
    delete a; delete b; delete c;
    return 0;
}

How I can delete the allocated sum_obj object?

Can using smart pointers help here?

Notice: I am doing this object for heavy math calculations, please don't suggest using stack memory.

Any reference or helpful answer would be appreciated.

  • You cannot, one the `operator+` exits the pointer to the object has been lost. Instead you should not allocate it in the first place. `JMP operator+(JMP &j) { return JMP("0"); }` does the same as your code without the unnecessary allocation. – john May 02 '23 at 11:54
  • Don't do this. `JMP *sum_obj = new JMP("0"); return *sum_obj;` can be replaced with `return JMP("0");` – NathanOliver May 02 '23 at 11:54
  • If you use a smart pointer(for example `std::shared_ptr` or `std::unique_ptr`), it will handle the deletion automatically for you – Karen Baghdasaryan May 02 '23 at 11:55
  • Sometimes developers who know other languages, don't appreciate that in C++ you don't have to use `new` for everything, and that it's perfectly acceptable to copy objects not pointers. – john May 02 '23 at 11:56
  • This object is a heavy object for heavy math calculations and it is not logical for me to use the stack memory, friends –  May 02 '23 at 11:57
  • @JibelSadeghi Then use a smart pointer, as already suggested. But in any case `return JMP("0");` does not copy anything. You might want to check out return value optimization (RVO). – john May 02 '23 at 11:57
  • but you're already making a with your current code and have 3 objects on the stack. – NathanOliver May 02 '23 at 11:58
  • @john Thank you, but can you tell me exactly how I should use the smart pointer in such a code so that it automatically deletes itself? –  May 02 '23 at 11:58
  • You cannot switch to a smart pointer in that code (I mean `operator+`). You have to change all your code so that is uses smart pointers instead of `JMP`. e,g, `std::shared_ptr`. Or change `JMP` so that internally it uses smart pointers and is therefore cheap to copy. – john May 02 '23 at 11:59
  • *it is not logical for me to use the stack memory* In your `main` routine, you have JMP a, b and c which are all stack memory. – Eljay May 02 '23 at 12:01
  • There's no quick fix here, for your `operator+` to return a reference is fundamentally flawed. You have to make a decision, use values, with the copying that involves (although RVO and move semantics means that should be less than you anticipate) or switch entirely smart pointers and all that entails. – john May 02 '23 at 12:03
  • @john I wrote a very simple and short code for this so that if someone like you has an idea from scratch, he can implement it. If you can, please provide your suggestion for using smart pointers in the answer section. –  May 02 '23 at 12:09
  • @JibelSadeghi Now, after your edit, you will remove `sum_obj`, but you will always leak `c`. – Yksisarvinen May 02 '23 at 12:52
  • @Yksisarvinen How to escape the `c` leak? –  May 02 '23 at 12:53
  • @JibelSadeghi Sorry but the question is now closed. But smart pointers are not hard. Just forget about using pointers and references and replace `JMP*` or `JMP&` with `std::shared_ptr`, replace `new JMP(...)` with `std::make_shared(...)`. That's more or less all you need to do. – john May 02 '23 at 13:08
  • Also, the `std::string` is itself a kind of smart pointer, specialized for string operations, which is heap allocated. (There's a short string optimization, which your std::string may or may not have. But that's irrelevant for longer strings.) – Eljay May 02 '23 at 13:37
  • Its ok to worry about performance, but adding `new` and raw pointers will not make your code magically run faster, but certainly it will introduce bugs. – 463035818_is_not_an_ai May 02 '23 at 13:38
  • An `operator+()` normally returns an object by VALUE, not by reference. You wouldn't expect `c = a + b` to cause a memory leak, regardless of the types of `a`, `b`, and `c`. One possible definition would be `JMP operator+(const JMP &rhs) const {JMP retval("0"); retval.number[0] += 1; return retval;}` Also, in your code, `delete a,b,c;` is incorrect and (presumably, assuming you want `a`, `b,` and `c` to all be deleted) should be `delete a; delete b; delete c`. – Peter May 02 '23 at 13:38
  • C++ was the first language to introduce the `new` keyword, but it is also the language that uses the keyword very differently than others. A good design should ideally not involve any appearence of this keyword in user code; Unless some very complicated, higly hand-optimized data-structure is involved; very unlikely in majority of applications. Reputable Libraries use it on behalf of user code, but the implementors know exactly what they are doing. – Red.Wave May 02 '23 at 20:56

1 Answers1

0

First, returning a reference is wrong. You need to return a value.

Secondly, the obvious solution is to not use dynamic memory at all, e.g.:

class JMP
{
    public:
        std::string number;
        JMP() {}
        JMP(const char* num) { number = num; }
        JMP operator+(const JMP &j)
        {
            JMP sum_obj("0");
            sum_obj.number[0] += 1;
            return sum_obj;
        }
};

Though, if you really need to do calculation with a heap object, you can do it like this:

#include <memory>

class JMP
{
    public:
        std::string number;
        JMP() {}
        JMP(const char* num) { number = num; }
        JMP operator+(const JMP &j)
        {
            // Allocate object
            auto sum_obj = std::make_unique<JMP>("0");
            sum_obj->number[0] += 1;
            return *sum_obj; // returns a copy
        }
};

Note that the heap solution is more costly. Both becauase using dynamic memory has a cost in itself, but also since it prevents return-value-optimization.

Edit: In the unique_ptr solution, it actually turns the copy into a move if changing the return line to: return std::move(*sum_obj);:

See working code, and difference depending on which return line that is commented out: Godbolt

cptFracassa
  • 893
  • 4
  • 14
  • Does the second code delete the `sum_obj` after returning it to the heap memory? –  May 02 '23 at 13:06
  • 1
    Yes Jibel, that is the point of it being a unique_ptr. It deletes its content when going out of scope, i.e. at the end of the `operator+` function. – cptFracassa May 02 '23 at 13:18
  • It is rarely necessary to dynamically create an object, just to return a copy of that object, and then release the dynamically allocated object. More often than not, a requirement to do that is a sign that some aspect of the class design is badly broken - and the better approach is to fix the design, not work around it with dynamic allocation, copy, and release. – Peter May 02 '23 at 13:42
  • @Jibel This code is correct (I think) but it does copy the `JMP` object when it returns from `operator+`, which I believe was what you were trying to avoid. – john May 02 '23 at 13:51
  • Yes, can i avoid from copying with `std::move()` in this code? –  May 02 '23 at 13:59
  • @Jibel Yes, you turn the copy into a move instead, see the link I edited into the answer. – cptFracassa May 02 '23 at 14:14