Program received signal SIGSEGV, Segmentation fault. C++ LIST

Question

L.insert() gives segmentation fault if I call it after L.deleteElement(). This is the error message:

196     nPtr = (nodePtr)malloc(sizeof(node));
(gdb) print(c)
$7 = 2
(gdb) next
197     if(nPtr!=NULL){
(gdb) print(nPtr)
$8 = (nodePtr) 0x615c70
(gdb) next
198         nPtr->data = element;
(gdb) print(element)
$9 = "("
(gdb) next

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b74413 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_assign(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6

I created a new node and didn't dereference any uninitialized pointer! Can anyone explain why I get this error? And it works as expected until I call it after L.deleteEle().

Here is my full prog:

#include<iostream>
#include<stdlib.h>
#include<string>
using namespace std;
struct node
{
    string data;
    struct node *next;
};
typedef struct node node;
typedef node* nodePtr;
class List
{
    nodePtr head;
    int count;
    nodePtr tail;
public:
    List(nodePtr h=NULL,int c=0,nodePtr t=NULL){ // constructor
        head=h;
        count=c;
        tail=t;
    } 
    bool insert(string element, int position=1); // if position is invalid, returns false ; takes default position as START (1)
    bool deleteEle(int position); // deletes the node at specified position, else returns false.
};

bool List::deleteEle(int position)
{
    if(position>count){
        return false;
    }
    nodePtr pPtr;
    nodePtr cPtr;
    nodePtr tPtr;
    int p=position;
    if(position==1){
        cPtr=head;
        tPtr=cPtr->next;
        head=tPtr;
        free(cPtr);
        count-=1;
        return true;
    }
    cPtr=head;
    pPtr=NULL;
    while(p>1){
        pPtr=cPtr;
        cPtr=cPtr->next;
        p--;
    }
    if(position==count){
        tail=pPtr;
        tail->next=NULL;
        free(cPtr);
        count-=1;
        return true;
    }
    tPtr=cPtr->next;
    pPtr->next=tPtr;
    free(cPtr);
    count-=1;
    return true;
}
bool List::insert(string element, int position)
{
    if (position>count+1){
        return false;
    }
    int c = position;
    nodePtr nPtr;
    nodePtr tPtr;
    nodePtr cPtr;
    nPtr = (nodePtr)malloc(sizeof(node));
    if(nPtr!=NULL){
        nPtr->data = element;
        nPtr->next = NULL;

        tPtr = NULL;
        cPtr = head;
        if(cPtr==NULL){
            head=nPtr;
            tail=nPtr;
            count+=1;
            return true;
        }
        else if(position==count+1){
            cout<<"ikkade !!!";
            tail->next=nPtr;
            tail=nPtr;
            count+=1;
            return true;
        }
        else if(position==1){
            head=nPtr;
            nPtr->next=cPtr;
            count+=1;
            return true;
        }
        else{
            while(cPtr!=NULL && c>2){
                cPtr = cPtr->next;
                c--; 
            }
            tPtr=cPtr->next;
            cPtr->next=nPtr;
            nPtr->next=tPtr;
            count+=1;
            return true;
        }
    }
    else{
        cout<<element<<" not inserted! No memory available.";
        return false;
    }
}
int main(void)
{
    List L;
    L.insert("(",L.size()+1);
    L.insert("+",L.size()+1);
    L.deleteEle(L.size());
    L.insert("(",L.size()+1); //fails here
return 0;       
}

Insert fails after calling deleteEle!

Answer 1

One issue that can cause a segmentation fault is the (mis)use of malloc and free in a C++ program to create objects dynamically.

This struct:

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

and then later code doing this:

nPtr = (nodePtr)malloc(sizeof(node));

does not create node objects, since the constructor for node is not invoked, thus the default constructor for std::string is not invoked.

All the malloc does is allocate sizeof(node) bytes -- no node object is actually created. You are now left with a bunch of allocated bytes just laying around that do not form any sort of node object whatsoever. Any use of nPtr as if it is a valid object invokes undefined behavior.

Instead of malloc and free, use new and delete, since new calls the constructor for the object, while malloc doesn't, and in addtion, delete calls the destructor while free does not.

So replace the malloc and free calls you're making now with the following (of course, change the names of the variables where necessary):

nPtr = new node;

and then when done:

delete nPtr;

---

Now malloc has its uses when creating an object -- it can be used for things such as placement-new, where you would issue a call to malloc, and then use that to create the object. But obviously your code is not using placement-new.