Segmentation fault along with core dumped after running

[twownage15]

So I still can't seem to find this seg fault when compiling on ubuntu. I ran valgrind and appears it's coming from this function. What is a good way to find these? I tried drawing it out on paper but couldn't seem to pick up my pointer errors. Here is my code, any advice is much appreciated, thanks!

  bool Ordered_list::remove(int data){

if(list == NULL)
	return false;
// if head node is has data equal to data then forward head
if(list->data == data){
		node *temp = list;
		list = list->next;
		delete temp;
		return true;
}else{
		node *temp = list;
		while(temp->next != NULL){
	if(temp->next->data == data){ // if temp next data is equal to data then remove it
		node *temp = temp->next;
		temp->next = temp->next->next;
		delete temp;
		return true;
	}

	temp = temp->next;
}

	return false; // data is not available in list
	}
}

[helios]

Line 17: It looks like you meant delete temp->next.

By the way, fix that formatting. It's almost impossible to read.

[Kourosh23]

@townage15 and @helios, I am interested to learn this notation ->
What is it called and what is it more often used for ? If you could share some of your knowledge it would be great. Thanks

[mbozzi]

Indirect member access.
http://en.cppreference.com/w/cpp/language/operator_member_access

Consider a structure

struct foo { int member; } bar;
// To access bar's member, I write:
bar.member;
// If I have a pointer to foo:
foo* baz = &bar; 
// I can access the pointed-to object's member using
baz->member;

[helios]

Whoops. My bad.
Avoid doing this:

T name;
{
     T name;
}

What you should have done is

node *temp2 = temp->next;
temp->next = temp->next->next;
delete temp2;

Personally, I would rename 'temp' to 'current' and 'temp2' to 'temp'.

[TheIdeasMan]

Just a quick question: why are you using delete on something you did not create with new?

http://en.cppreference.com/w/cpp/language/delete

Even if node's constructor uses new, would it still a better idea to not use new or delete at all, use std::unique_ptr instead?

http://en.cppreference.com/w/cpp/memory/unique_ptr

Also don't use NULL when you should be using nullptr