-1

After a few years, I discovered a memory leak bug in my code. Unfortunately the bug was not causing any issues to be noticed until I found out about it indirectly.

Below is a function addElement() from class c_string which adds a new element to a chain. My old way of doing it is like this:

class c_string
{
private:
    char *chain;
    size_t chain_total;
public:
    void addElement(char ch);
};


void c_string::addElement(char ch)
{
    char *tmpChain = new char[chain_total+1];

    for(size_t i=0;i<chain_total;i++)
        tmpChain[i] = chain[i];
    tmpChain[chain_total++] = ch;

    delete chain;
    chain = new char[chain_total];
    chain = tmpChain;
}

I found out that by doing chain = tmpChain; I am causing a memory leak. My question is, what is the best way to handle it?

Please note, in this class I am not using any STL function, because I am simply writing my own string.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Ninja
  • 59
  • 5
  • 2
    Do you just re-ask same question? [A question about dynamic memory allocation](https://stackoverflow.com/questions/73170389/a-question-about-dynamic-memory-allocation) – apple apple Jul 30 '22 at 19:30
  • 1
    `delete[] chain; chain = tmpChain;` – beothunder Jul 30 '22 at 19:32
  • 2
    The best way to do it is not to even do it in the first place. With modern C++, containers completely eliminate the need to allocate and properly manage all of the memory yourself. None of the functionality in the shown code requires anything to be `new`ed or `delete`d. Everything can be done using the appropriate containers, which contain 100% bug-free code that pertains to memory allocations. The best way to avoid bugs and memory leaks in C++ is by making them logically impossible. – Sam Varshavchik Jul 30 '22 at 19:32
  • 1
    `chain = new char[chain_total];` this line is useless and causing the memory leak – Ashish Chourasia Jul 30 '22 at 19:33
  • Thank you for your answer @ apple apple. No this is another question, the first i learned about memory leak, now i want to know if there is any better way to rewrite my function – Ninja Jul 30 '22 at 19:33
  • Thank you @Sam Varshavchik, but i mentioned that i am doing my own string)) – Ninja Jul 30 '22 at 19:38
  • @ Ashish Chourasia, I do not think so since I already deleted chain before that line – Ninja Jul 30 '22 at 19:39
  • 1
    *"I am causing a memory leak. My question what is the best way to do it?"* -- the best way to cause a memory leak? I think you've abused "it" here and should try being more verbose to be clearer. – JaMiT Jul 30 '22 at 20:16
  • @Ninja *"but i mentioned that i am doing my own string"* -- perhaps you should implement your own smart pointer before implementing your own string? One cause of the issue you are facing is that your one class is responsible for multiple tasks -- memory management (delegate to a smart pointer) and memory (string) content. – JaMiT Jul 30 '22 at 20:20
  • Thanks @JaMiT ) this is only a fragment of the whole codes) – Ninja Jul 31 '22 at 21:26

1 Answers1

1

The best way to do it is simply drop the second allocation, it serves no purpose.

void c_string::addElement(char ch)
{
    char *tmpChain = new char[chain_total+1];

    for(size_t i=0;i<chain_total;i++)
        tmpChain[i] = chain[i];
    tmpChain[chain_total++] = ch;

    delete[] chain;
    chain = tmpChain;
}

The above is correct and even has the strong exception guarantee.

Of course even if you do not want to use std::string, std::unique_ptr would is still safer than raw new+delete and you would get the rule of five for free instead of having to implement it on your own.

From performance standpoint, you might be interested in Why is it common practice to double array capacity when full? and of course std::memcpy or std::copy instead of the manual loop.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Quimby
  • 17,735
  • 4
  • 35
  • 55
  • can you please explain "The above is correct and even has the strong exception guarantee." – Ninja Jul 30 '22 at 19:41
  • 2
    If you go to www.google.com, type "exception guarantee c++" you will get a complete explanation. Google is pretty good at these kinds of things... – Sam Varshavchik Jul 30 '22 at 19:42
  • 2
    @Ninja Strong exception guarantee briefly means that if the method throws for whatever reason (e.g. `new` can throw), the observable state of the object(`this`) is unchanged. The operation can either succeed or do nothing. – Quimby Jul 30 '22 at 19:46