-1

I'm solving the following exercise (17.4) from Stroustrup's PPP book:

Write a function char* strdup(const char* ) that copies a C-style string into memory it allocates on the free store. Don't use any standard library function.

Here's my implementation, which compiles just fine. I have a question about an error message that I found when I run the function.

#include <iostream>
#include <string>
char* strdup(const char* s) {
    if (s==0) return 0;

    // get number of char in s
    int n = 0;
    while (s[n] != 0)
        ++n;

    // allocate memory with room for terminating 0
    char* pc = new char[n+1];

    // copy string
    for (int i = 0; s[i]; ++i)
        pc[i] = s[i];
    pc[n] = 0;  // zero at the end: it's a C-style string

    delete[] s;
    return pc;
}

int main()
try {
    std::string str;
    char* cstr;
    while (std::cin>>str && str!="quit") {
        cstr = strdup(&str[0]);
        std::cout << cstr << "\n";
        delete[] cstr;
    }
}
catch (std::exception& e) {
    std::cerr << "exception: " << e.what() << std::endl;
}
catch (...) {
    std::cerr << "exception\n";
}

It compiles, but when I run it and I write the first character, I have a pointer being freed was not allocatederror. If I remove delete[] s, then I have no memory leak and it runs just fine. But why is (apparently) correct to do not delete[]the s? Is it because it has not been allocated with new?

cnewbie
  • 177
  • 5
  • 1
    `delete[] s;` what? why? did Stroustrup ask you to copy a string and then delete the original? what's the point of such copying? and of course you don't know whether the original is allocated on the free store, so you cannot even *think* about deleting it. – n. m. could be an AI Sep 22 '21 at 09:38

1 Answers1

1

A std::string does manage the memory it uses to store the string. In main you do

 std::string str;

and

cstr = strdup(&str[0]);

but your strdup calls delete[] s; on the parameter.

This is what you already know. Now consider that the destructor of std::string does already clean up the memory when it goes out of scope. The buffer used by the std::string cannot be deleted twice. You shall not call delete on &str[0]. You only need to delete objects that you created via new.

Also there is small string optimization. In this case &str[0] does not point to a heap allocated buffer which you could delete.

PS: You are using 0 when you should rather use nullptr for pointers and '\0' for the null terminator.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • Thanks for you answer. So, modulo you P.S., it's sufficient to remove the line with `delete[] s` to have the right solution, right? – cnewbie Sep 22 '21 at 09:42
  • @bobinthebox yes, just remove it. i didnt find any other issue. – 463035818_is_not_an_ai Sep 22 '21 at 09:43
  • Thanks. So the `pointer being freed was not allocated` means: "I am deleting a pointer which has already been freed by the destructor of `std::string`, in this particular case"? – cnewbie Sep 22 '21 at 09:44
  • @bobinthebox see edit, it could also mean that `&str[0]` does not actually point to memory that was allocated via `new`. – 463035818_is_not_an_ai Sep 22 '21 at 09:46
  • One last thing: the destructor of `s`is invoked whenever the object goes out of scope. But when does `str` goes out of scope, in my code? – cnewbie Sep 22 '21 at 10:19
  • @bobinthebox at the end of `main` at the last `}` – 463035818_is_not_an_ai Sep 22 '21 at 10:20
  • Ok, so when I pass `&str[0]` as an argument of my function, the destructor of `s` is not invoked after the function is executed. This is because I'm passing a pointer, so the memory address, right? – cnewbie Sep 22 '21 at 10:45
  • @bobinthebox comments arent the place to explain this. I am certain Bjarne will explain it in all details in the book – 463035818_is_not_an_ai Sep 22 '21 at 10:46
  • Honestly I tried, but I couldn't find it at the moment. I mean, by passing a pointer we're sure that the object pointed to is not freed after the function call ends, right? – cnewbie Sep 22 '21 at 10:50