Error spotted in C++ Primer 5th edition shared_ptr

Question

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?

Answer 1

As you rightfully pointed out, the text description, and the comments in the code both do not fit the code. They are more in line with code like this:

shared_ptr<int> p(new int(42));// reference count is 1
{//new block started
    shared_ptr<int> q(p.get());
}// 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

If I were to guess, I would say this is how the example first looked like, and then somebody decided to introduce the raw pointer without realising it would also need changes to the comments and text.

Answer 2

In sixth printing, the contents that you indicated have been slightly different 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
    // undefined: two independent shared_ptrs point to the same memory
    auto local = shared_ptr<int>(q);
} // block ends, local is destroyed, and the memory to which p and  q points is freed
int foo = *p; // undefined; the memory to which p points was freed

Here, p, q, and local all point to the same memory. Because p and local were created independently from one another, each has a reference count of 1. When the inner block ends, local is destroyed. Because local’s reference count is 1, the memory to which it points will be freed. That makes p and q into a dangling pointers; what happens when we attempt to use p or q is undefined. Moreover, when p is destroyed, the pointer to that memory will be deleted a second time.

I think that the author's errata page isn't managed any more.

Answer 3

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.