Search code examples
c++c++11c++14shared-ptrerrata

Error spotted in C++ Primer 5th edition shared_ptr<int>


Hi i am reading C++ primer 5th edition and i think i have spotted one error under the section shared_ptr. First i am writing the code and the explanation that they have given. Then i will write what i think is the error and what i think is actually happening. The code is as follows:

shared_ptr<int> p(new int(42));// reference count is 1
int *q = p.get();// ok: but don't use q in any way that might delete its pointer
{//new block started
    shared_ptr<int>(q);
}// block ends, q is destroyed, and the memory to which q points is freed
int foo = *p;// undefined; the memory to which p points was freed

The explanation they have given is as follows:

In this case, both p and q point to the same memory. Because they were created independently from each other, each has a reference count of 1. When the block in which q was defined ends, q is destroyed. Destroying q frees the memory to which q points. That makes p into a dangling pointer, meaning that what happens when we attempt to use p is undefined. Moreover, when p is destroyed, the pointer to that memory will be deleted a second time.

Now i think the error is the statement "When the block in which q was defined ends, q is destroyed.Destroying q frees the memory to which q points." and also the reasoning they gave behind why p is a dangling pointer is faulty. Below is my reasoning why p is a dangling pointer and why first quoted statement is an error.

  1. When the block in which q was defined ends, q is destroyed. But the memory to which q points is not freed since q is a builtin pointer and not a shared_ptr. And unless we explicitly write delete q the corresponding memory will not be freed.
  2. Now, inside the new block we have created a temporary shared_ptr using q. But this temporary is independent of p. and so when this inner block ends the temporary is destroyed and hence the memory is freed. But note that p still point to the same memory which has been freed. So p is now a dangling pointer and using p in the statement int foo=*p is undefined.

I think this is the correct explanation of why p is a dangling pointer and also the correction that should be there. Can someone confirm if this is right or am i doing something wrong?


Solution

  • C++ Primer 5th edition as well as your explanation has made one common mistake while trying to explain this program. Note that the statement:

    shared_ptr<int>(q);
    

    creates a new temporary named q instead of creating a new temporary using q as parameter to a shared_ptr's constructor. The below example code shows this:

    #include <iostream>
    
    using namespace std;
    struct NAME
    {
        int p = 0;
        NAME()
        {
            std::cout<<"default constructor"<<std::endl;
        }
        NAME(int d): p(d)
        {
            std::cout<<"d: "<<d<<" p: "<<p<<std::endl;
           
            
        }
        NAME(const NAME& n)
        {
            std::cout<<"const copy constructor"<<std::endl;
        }
        NAME(NAME& n)
        {
            std::cout<<"non const copy constructor"<<std::endl;
        }
        ~NAME(){
        std::cout<<"destructor: "<<p<<std::endl;
        
        }
    };
    int main()
    {
       cout << "Hello World" << endl; 
       NAME (4);//calls converting constructor
       //after the completion of the above full statement the temporary is destroyed and hence you get a destructor call in the output
       
       NAME k;//calls default constructor
       std::cout<<"k.p: "<<k.p<<std::endl;
       
       NAME(l);//this creates a temporary named l instead of creating a temporary using l as parameter to NAME's constructor ( in particular,default constructor)
        
       NAME{l};//this creates a temporary using l as parameter to NAME's constructor with non-const copy constructor
        
    
       
       return 0;
    }
    

    The 2nd point of your explanation is correct only if we use shared_ptr<int>{q}; instead of shared_ptr<int>(q);.

    This means:

    1. Since the author has used shared_ptr<int>(q); a local variable named q is created which is a smart pointer instead of a built in pointer from the outer scope.

    2. This local variable q has nothing to do with both p or q from the outer scope and hence when this local q goes out of scope shared_ptr's destructor is called. But note the memory to which the outer p or q points is not freed.

    3. So afterwards when we write int foo = *p; there is no undefined bahavior.

    Below is the code that shows that shared_ptr<int>(q); defines a local named q instead of a temporary using q as parameter.

    #include <iostream>
    #include <memory>
    using namespace std;
    
    int main()
    {
       cout << "Hello World" << endl; 
      
       
    shared_ptr<int> p(new int(42)); // reference count is 1
    int *q = p.get();  // ok: but don't use q in any way that might delete its pointer
    std::cout<<"q's address "<<&q<<std::endl;
    std::cout<<"p's address "<<&p<<std::endl;
        { // new block
        
        shared_ptr<int>(q);
        std::cout<<"new q's address "<<&q<<std::endl;
        std::cout<<"new q's value "<<(*q)<<std::endl;//this produces segmentation fault
    } // block ends, local is destroyed
    int foo = *p; // this is ok
    
       return 0;
    }
    

    In the above code if we try to access local q's value using *q then we will get undefined behavior(which may crash the program/segmentation fault) since we are dereferencing a null pointer. If we remove this *q then the program have no undefined behavior.

    Now even in the next edition of the book(6th edition) the author is using auto local = shared_ptr<int>(q); when he could have just used shared_ptr<int>{q}; to make his point.