12

I have only been using raw pointers for linked list with templates. For example, the member data, Node<T>* head; and when I am inserting a node one of the lines would be head = new Node<T>(data);.

However, now I need to use a smart pointer and I am not sure how I would change it to use smart pointers. Would the member data be changed to shared_ptr<Node<T>> head; and the other line would change to
head = shared_ptr<Node<T>>( new <Node<T>>(data) );?

Mark
  • 283
  • 1
  • 2
  • 14
  • 1
    Not sure why you want a shared ptr instead of a unique ptr, but yes. Also, assuming you're using c++14, you should use std::make_unique/make_shared. You'll need to be very careful when adding/removing elements that you don't accidentally delete the tail of your list, though. – xaxxon Apr 17 '16 at 06:35
  • 1
    Be careful using smart pointers for linked lists, since the destructors will end up being called recursively when you delete any node in the list that contains another node; this can cause a stack overflow if the deleted part of the list links enough elements. – Matt Jordan Apr 17 '16 at 06:42
  • @xaxxon Is unique ptr better to use here? If I change it, would I just need to change all the `shared_ptr` parts to become `unique_ptr`? – Mark Apr 17 '16 at 06:45
  • @xaxxon I was able to change everything to using shared ptr, but I am getting a lot of memory leaks shown by the debugger. Is this why unique ptr is a better option? – Mark Apr 17 '16 at 06:59
  • 3
    @Mark We have no idea, since we're not mind readers and cannot see your code. When used *correctly*, smart pointers assist to *prevent* memory leaks; not introduce them. – WhozCraig Apr 17 '16 at 07:07
  • @Mark are you allocating the shared_ptr on the heap. The destructor of shared_ptr must be called to remove a reference of the shared_ptr... – benzeno Apr 17 '16 at 09:21
  • In theory, shared_ptr allows for sharing structure easily. This way you don't need your `cdr` to be a deep copy. – bipll Apr 17 '16 at 11:37

6 Answers6

21

You do not "need" to use a smart pointer for a linked list, because that statement doesn't make sense. You do not use smart pointers for low-level data structures. You use smart pointers for high-level program logic.

As far as low-level data structures are concerned, you use a standard container class from the C++ standard library, like std::list [*], which solves all your memory-management problems anyway, without using any smart pointers internally.

If you really really need your own highly specialised/optimised custom container class because the entire C++ standard library is unfit for your requirements and you need a replacement for std::list, std::vector, std::unordered_map and other optimised, tested, documented and safe containers – which I very much doubt! –, then you have to manage memory manually anyway, because the point of such a specialised class will almost certainly be the need for techniques like memory pools, copy-on-write or even garbage collection, all of which conflict with a typical smart pointer's rather simplistic deletion logic.

In the words of Herb Sutter:

Never use owning raw pointers and delete, except in rare cases when implementing your own low-level data structure (and even then keep that well encapsulated inside a class boundary).

Something along those lines is also expressed in Herb Sutter's and Bjarne Stroustrup's C++ Core Guidelines:

This problem cannot be solved (at scale) by transforming all owning pointers to unique_ptrs and shared_ptrs, partly because we need/use owning "raw pointers" as well as simple pointers in the implementation of our fundamental resource handles. For example, common vector implementations have one owning pointer and two non-owning pointers.

Writing a linked-list class in C++ with raw pointers can be a useful academic exercise. Writing a linked-list class in C++ with smart pointers is a pointless academic exercise. Using any of these two self-made things in production code is almost automatically wrong.


[*] Or just std::vector, because due to cache locality that will almost always be the better choice anyway.

Christian Hackl
  • 27,051
  • 3
  • 32
  • 62
  • I think this is the best answer... it was what I was trying to get to in my answer. I just couldn't get to it in a non-roundabout way. To keep the raw pointers in the class boundary I believe the OP would need to delete the underlying data in the Node destructor and iteratively delete Nodes in the LinkedList destructor... right? – benzeno Apr 17 '16 at 14:22
  • @benzeno: Class boundary refers more to the fact that users of the class should not be aware of its implementation; i.e. no `T*` appears in the public interface of the class. Whether `delete` is even necessary depends on the implementation of the container. Some of the advanced techniques I listed as examples work without explicit `delete`s. Then again, given `std::list`, a simple linked list does not require advanced techniques or any custom class at all. – Christian Hackl Apr 17 '16 at 15:18
  • @benzeno: Other than that, yes, a destructor of such a simple custom linked list would do that. – Christian Hackl Apr 17 '16 at 15:19
  • 1
    I would disagree with the statement "don't use smart pointers for low-level data structures" (... and Herb Sutter doesn't state that either). Consider a binary tree, for example: When you got endless ressources, you set up a standard-library like implementation using raw pointers under the hood as well as all the sophisticated stuff you mentioned. But in reality, you don't have these ressources, and then there's no reason not to go with `unique_ptr` -- you will usually lose hardly any performance with that and gain a lot of safety. – davidhigh Apr 17 '16 at 21:12
  • 5
    This is not an answer to the question, therefore -1. You can have all the opinions you want about the question, but here you should first and foremost give an answer to it. I would also give you one more -1 if I could, for the unneeded boldness. – Fabio A. Oct 28 '19 at 21:37
  • @FabioA.: Answering a beginner's questions *literally*, like a robot, is almost always a disservice both to the beginner and to Stack Overflow as a whole. You must instead consider the broader context of a question. Trust me, I've trained juniors and tutored my share of freshmen at university. Funny enough, the OP seems to agree! As for the boldness, it is a standard UX technique to make text more readable; sometimes it's even used in print (Joshua Bloch's seminal "Effective Java" comes to mind). Of course, you are entitled to downvote anything for whatever reasons you feel are appropriate. – Christian Hackl Oct 28 '19 at 21:51
  • 4
    @ChristianHackl, I am very far from being a beginner, yet I arrived here just because I was curious about the same topic as the OP's, only to find your answer which isn't an answer at all. There **are** legitimate uses for what the OP's asked for, therefore your statement by which _«You do not "need" to use a smart pointer for a linked list, because that statement doesn't make sense.»_ is unfactual and unnecessarily _bold_. "bold" there refers to the manner, not to the **text style**. – Fabio A. Nov 01 '19 at 21:43
  • @FabioA.: You may be curious about the topic as much as you like, and I certainly have no problem with that, but you still do not "need" to use a smart pointer for a linked list, because that statement doesn't make sense, as linked lists are best implemented without smart pointers. There are no legitimate uses for what the OP's asking that I can think of. Prove me wrong. – Christian Hackl Nov 01 '19 at 22:49
17

There are basically two alternatives to set up a smart-pointer enhanced list:

  1. Using std::unique_ptr:

    template<typename T>
    struct Node
    {
         Node* _prev;
         std::unique_ptr<Node> _next;
         T data;
    };
    
    std::unique_ptr<Node<T> > root; //inside list
    

    That would be my first choice. The unique-pointer _next takes care there are no memory leaks, whereas _prev is an observing pointer. However, copy constructor and such things -- in case you need them -- need to be defined and implemented by hand.

  2. Using shared_ptr:

    template<typename T>
    struct Node
    {
         std::weak_ptr<Node> _prev;   //or as well Node*
         std::shared_ptr<Node> _next;
         T data;
    };
    
    std::shared_ptr<Node<T> > root; //inside list
    

    This is alternative is copyable by design and adds further safety because of the weak_ptr, see below. It is less performant than the unique_ptr when it comes to structural changes of the list, such as insertions and removals, e.g. due to thread safety in shared_ptr's control block.

    Yet, traversing the list, i.e. dereferencing the pointers, should be as performant as for the unique_ptr.

In both approaches the idea is that one node owns the complete remaining list. Now when a node goes out of scope, there is no danger that the remaining list becomes a memory leak, as the nodes are iteratively destructed (starting from the last one).

The _prev pointer is in both options only an observing pointer: it's task is not to keep the previous nodes alive, but only to provide a link to visit them. For that, a Node * is usually sufficient (--note: observing pointer means you never do memory related stuff like new, delete on the pointer).

If you want more safety, you can also use a std::weak_ptr which prevents from things like

std::shared_ptr<Node<T> > n;
{
    list<T> li;
    //fill the list
    n = li.root->next->next; //let's say that works for this example
}
n->_prev; //dangling pointer, the previous list does not exists anymore 

Using a weak_ptr, you can lock() it and in this way chack whether _prev is still valid.

davidhigh
  • 14,652
  • 2
  • 44
  • 75
  • 1
    A bit of time has passed since the answer ... but I'm curious how your unique_ptr<> version works, as it would be your first choice. The unique_ptr<> of the current node is owning the memory for the next node, so when you delete the current node by key for example you will deallocate the memory for the next node also ... you will suffer from the same problems as https://stackoverflow.com/questions/35535312/stack-overflow-with-unique-ptr-linked-list – celavek Oct 04 '17 at 13:36
  • @cevalek: fair point, I never used the above approach for such huge trees. However, as opposed to the problems for a list in your link, for trees it seems to be even worse. The reason is that you usually work iteratively on trees, and when you get a stack-overflow by iteratively calling the destructor, you probably also can't call any other function recursively. – davidhigh Oct 04 '17 at 16:06
  • @cevalek: A possible workaround would be to traverse from leaf nodes upwards -- one could implement that in the `Node` desctructor, but that would destroy the advantages of the unique-ptr. So possibly it would be best to implement a custom unique-ptr -- one that assumes a tree structure and deletes from downward up. (But that's just some quick suggestions). Anyways, this whole thing is no argument for using a raw pointer instead of a smart pointer ... just to mention it. – davidhigh Oct 04 '17 at 16:07
1

I would look at the interface of std::list, which is a C++ implementation of linked lists. It seems that you are approaching the templating of your Linked list class wrong. Ideally your linked list should not care about ownership semantics (i.e. whether it is instantiated with raw ptrs, smart pointers or stack allocated variables). An example of ownership sematics with STL containers follows. However, there are better examples of STL and ownership from more authoritative sources.

#include <iostream>
#include <list>
#include <memory>

using namespace std;

int main()
{

    // Unique ownership.
    unique_ptr<int> int_ptr = make_unique<int>(5);

    {
        // list of uniquely owned integers.
        list<unique_ptr<int>> list_unique_integers;

        // Transfer of ownership from my parent stack frame to the
        // unique_ptr list.
        list_unique_integers.push_back(move(int_ptr));

    } // list is destroyed and the integers it owns.

    // Accessing the integer here is not a good idea.
    // cout << *int_ptr << endl;
    // You can make a new one though.
    int_ptr.reset(new int(6));

    // Shared ownership.
    // Create a pointer we intend to share.
    shared_ptr<int> a_shared_int = make_shared<int>(5);

    {
        // A list that shares ownership of integers with anyone that has
        // copied the shared pointer.
        list<shared_ptr<int>> list_shared_integers;

        list_shared_integers.push_back(a_shared_int);

        // Editing and reading obviously works.
        const shared_ptr<int> a_ref_to_int = list_shared_integers.back();
        (*a_ref_to_int)++;
        cout << *a_ref_to_int << endl;

    } // list_shared_integers goes out of scope, but the integer is not as a
    // "reference" to it still exists.

    // a_shared_int is still accessible.
    (*a_shared_int)++;
    cout << (*a_shared_int) << endl;

} // now the integer is deallocated because the shared_ptr goes 
// out of scope.

A good exercise to understand ownership, memory allocation/deallocation, and shared pointers is to do a tutorial where you implement your own smart pointers. Then you will understand exactly how to use smart pointers and you will have one of those xen moments where you realise how pretty much everything in C++ comes back to RAII (ownership of resources).

So back to the crux of your question. If you want to stick to Nodes of type T, don't wrap the node in a smart pointer. The Node destructor must delete the underlying raw pointer. The raw pointer may point to a smart pointer itself specified as T. When your "LinkedList"'s class destructor is called it iterates through all Nodes with Node::next and calls delete node; after it obtained the pointer to the next node.

You could create a list where nodes are smart pointers... but this is a very specialised linked list probably called SharedLinkedList or UniqueLinkedList with very different sematics for object creation, popping, etc. Just as an example, a UniqueLinkedList would move a node in the return value when popping a value to a caller. To do metaprogramming for this problem would require the use of partial specialization for different types of T passed. Example, something like:

template<class T>
struct LinkedList
{
    Node<T> *head;
};

// The very start of a LinkedList with shared ownership. In all your access
// methods, etc... you will be returning copies of the appropriate pointer, 
// therefore creating another reference to the underlying data.
template<class T>
struct LinkedList<std::shared_ptr<T>>
{
    shared_ptr<Node<T>> head;
};

Now you start implementing your own STL! You can already see potential for problems as mentioned in the comments to your question with this approach. If nodes have shared_ptr next it will result in a call to that shared Node's destructor, which will call the next shared Node destructor and so forth (stack overflow due to the recursion is possible). So that is why I don't care much for this approach.

benzeno
  • 661
  • 1
  • 6
  • 13
1

You should use a unique_ptr here as only the current node should own the next node. Introducing shared pointers not only hurts performance it introduces memory leak opportunities (this is the bigger concern) if you have bugs in your program. Here's an example implementation and as an added bonus I'm showing you how to reverse a linked list and how to iterate a linked list comprised of unique_ptrs:

struct Node{
    int val;
    unique_ptr<Node> next;
    Node(int v, unique_ptr<Node> n) : val(v), next(std::move(n)) {}
};

unique_ptr<Node> reverseList(unique_ptr<Node> head) {
    unique_ptr<Node> prev{nullptr};
    unique_ptr<Node> current = std::move(head);
    while (current) {
        unique_ptr<Node> temp = std::move(current->next);
        current->next = std::move(prev);
        prev = std::move(current);
        current = std::move(temp);
    }
    return prev;
}

void testReverseList() {
    auto n1 = make_unique<Node>(1, nullptr);
    auto n2 = make_unique<Node>(2, std::move(n1)); 
    auto n3 = make_unique<Node>(3, std::move(n2)); 
    auto n4 = make_unique<Node>(4, std::move(n3)); 
    auto n5 = make_unique<Node>(5, std::move(n4)); 

    Node* nodePtr = n5.get();
    cout << "before: " << endl;
    while(nodePtr) {
        cout << nodePtr->val << endl;
        nodePtr = nodePtr->next.get();
    }

    auto result = reverseList(std::move(n5));

    cout << "after: " << endl;
    nodePtr = result.get();
    while(nodePtr) {
        cout << nodePtr->val << endl;
        nodePtr = nodePtr->next.get();
    }
}

One thing to notice here is when we want to modify ownership of the pointers (like in the reverseList function) we need to pass in the unique_ptr and give it ownership of it, then we return the ownership of the new head. In other cases we can just use a raw pointer like we do for iterating to print the results.

UpAndAdam
  • 4,515
  • 3
  • 28
  • 46
jmbauerful
  • 34
  • 8
  • How does your answer provide information that isn't already provided among the many old answers already present and accepted. Your facts aren't even right, smart pointers provide much better safety than dumb pointers do, and that extends to shared pointers. There are seperate reasons why a shared pointer is a bad idea for a list, and in fact ANY smart pointer is bad for a list implementation. – UpAndAdam Jul 19 '23 at 16:04
  • @UpAndAdam Did you even read my answer or are you just downvoting everything I write? The other answers here say to use shared_ptr or unique_ptr and I’m proposing the right answer is unique_ptr as shared is not needed here…also I never said any facts about smart pointers vs dumb pointers so I don’t know what you’re talking about… Finally the question was about how to use a smart pointer in a linked list and not a dumb pointer so don’t try to tell me why we shouldn’t use smart pointers here as that is not what OP is asking – jmbauerful Jul 20 '23 at 05:46
  • 1
    Ah i see that nuanced difference now where you are pointing out unique vs shared. apologies. No I wasn't targeting you, I was reviewing the first answers queue yesterday. You answered an old question with an answer that appeared nearly identical to all the answers provided so I asked the question. You had a good answer to it.. I've now upvoted. Though your explanation of memory leaks with shared pointers is questionable since shared pointers are there to prevent the memory leaks dumb pointers provide. How does one leak memory with shared pointers? – UpAndAdam Jul 20 '23 at 15:34
  • actually one of the other answers DOES use unique pointers... – UpAndAdam Jul 20 '23 at 15:38
  • Thanks @UpAndAdam. Yeah the accepted answer I believe talked about shared and unique and I just wanted to call out why we should prefer unique here. Shared pointers are to help with memory leaks but you can still create cycles which would then create memory leaks so they aren’t as safe as unique pointers in that regard. – jmbauerful Jul 20 '23 at 20:15
0

Structure will look like

template<typename T> struct Node
{
    T data;
    shared_ptr<Node<T>> next;
};

Creating of node will look like

shared_ptr<Node<int>> head(new Node<int>);

or

auto head = make_shared<Node>(Node{ 1,nullptr });
UpAndAdam
  • 4,515
  • 3
  • 28
  • 46
Nancy Garg
  • 63
  • 8
  • Extremely preferred to use `make_shared` instead of calling new in this context as it provides exception safety that the new call does not – UpAndAdam Jul 20 '23 at 15:36
-2

dont use smart pointer in graph like data structure because it may cause stack overflow an many performance issue due to recursive call of destructor or inc, decr reference count wich it non optimal due to how dfs and bfs algorithm work

  • Your answer could be improved with additional supporting information. Please [edit] to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers [in the help center](/help/how-to-answer). – Community Apr 09 '22 at 00:36