I am trying to construct a linked list that contains string pointers. Since strings can be quite large, I am trying to save space and access time by storing pointers to them in the linked list. The strings are read from a text file and inserted into a linked list.
However, my insert function creates all nodes with pointers to a single memory address, so that all nodes contain a pointer to the same data. I am sure there must a relatively easy fix for this, but I can't seem to find it.
Please help!

Relevant code:

//node struct
struct sNode
		{
			string * data;
			sNode * next;
		};
//class definition
class stringList
{
	private:
		sNode * head;
		sNode * tail;
		int size;
	public:
		stringList();
		~stringList();
		bool listEmpty();
		void insertItem(string );
		int getPosition (sNode *);
		void deleteItem(int );
//functions relevant to insert operation
void stringList::insertItem (string s)
{
	if (listEmpty())
	{
		head=new sNode;
		head->next=NULL;
		head->data=&s;
		tail=head;
	}
	else
	{
	sNode * temp = new sNode;
	temp->data= &s;
	temp->next=NULL;
	tail=tail->next;
	}
	
	size++;
}

//client function to read from file
stringList s;
void readInput(ifstream & i)
{
string lineread;
while(std::getline(i, lineread))
	{
		s.insertItem(lineread);
	}
	
}

>>head->data=&s;

That's the problem. Don't do that in a linked list. Instead, let the linked list own the memory for all strings by removing the pointer

struct sNode
		{
			string  data;
			sNode * next;
		};

Thank you for the insight.
So is there really not any way that I can store pointers rather than strings? or are the pointers really too dangerous?
A colleague advised me that I should store string pointers for efficiency, so I would like to make that work if it is possible

>>A colleague advised me that I should store string pointers for efficiency

There is no efficiency gained by declaring pointers to std::string objects. In fact it will be less efficient because you have to allocate memory before it can be used.

It's somewhat different with C style character arrays. With them you have one of two choices
(1) declare the arrays to be a fixed size, which needs to be large enough to hold the longest string you might want to copy into it. That can waste a lot of memory of there are a lot of strings.
(2) declare a char pointer and allocate memory so that its only large enough to contain a specific string. Example: Note that you still have to duplicate the string. Somewhere in your program, either here or in the read() function new memory has to be allocated for each string read from the file.

struct node
{
    char* data;
    struct node* next;
}

void add(const string& d)
{
    struct node* newnode = new struct node;
    newnode->data = new char[d.length()+1]
    strcpy(newnode->data, d.c_str());
}

The std::string c++ class avoids making you, the programmer, deal with both the above two problems. So if you are using std::string then you don't have to worry about efficiency.

This question has already been answered. Start a new discussion instead.