CODEWITHSUNDEEP
c++linked-listsegmentation-fault
nan
nan

Reputation: 241

C++ strange segmentation fault

I am getting a strange segmentation fault in my program. Dlist is a class that creates a linked list with operations to insert and remove items from a dynamic list. I am positive my implementation of this class is correct, but this code is producing a seg fault. The strange part is, when I make my atleastTwo and atleastOne functions pass by reference, the seg fault disappears and everything compiles. Can anyone shed some light on this problem? 

bool atleastTwo(Dlist<double> stack){
try{
    stack.removeFront();
    stack.removeFront();
} catch(emptyList e){
    cout << "Not enough operands\n";
    return false;
}
return true;
}

bool atleastOne(Dlist<double> stack){
try{
    stack.removeFront();
} catch(emptyList e){
    cout << "Not enough operands\n";
    return false;
}
return true;
}

void processInput(inputs usrInput, Dlist<double> &stack){
switch(usrInput){
    case i_add:
        if(atleastTwo(stack)){doOperation(stack, add);}
        break;
    case i_subtract:
        if(atleastTwo(stack)){doOperation(stack, subtract);}
        break;
    case i_multiply:
        if(atleastTwo(stack)){doOperation(stack, multiply);}
        break;
    case i_divide:
        if(atleastTwo(stack)){doOperation(stack, divide);}
        break;
    case i_negation:
        if(atleastOne(stack))negation(stack);
        break;
    case i_duplicate:
        if(atleastOne(stack)){duplicate(stack);}
        break;
    case i_reverse:
        if(atleastTwo(stack)){reverse(stack);}
        break;
    case i_print:
        if(atleastOne(stack)){print(stack);}
        break;
    case i_clear:
        clear(stack);
        break;
    case i_printAll:
        printAll(stack);
        break;
    default:
        break;
}
}

T *removeFront();
// MODIFIES this
// EFFECTS removes and returns first object from non-empty list
//         throws an instance of emptyList if empty

Thanks

Upvotes: 0

Views: 222

Answers (3)

Andrei Tita
Andrei Tita

Reputation: 1236

After staring at your Dlist implementation code for quite a while (doesn't look wrong at a first glance), I think the problem is here:

template <typename T>
Dlist<T>::Dlist(const Dlist &l){
    removeAll();
    copyAll(l);
}

Calling removeAll() before initializing first (and last) to NULL will lead to bad things, given the operations which are performed there.

Another problem with the Dlist design is that you are holding raw pointers (which is tricky but not necessarily bad, assuming it's clear to everyone that the user is responsible for deleting them), yet you have implemented deep copy mechanics. Temporary Dlist objects will leak, as will the copies you are passing to your atLeastOne and atLeastTwo functions, unless you manually delete their contents.

Upvotes: 0

jdehaan
jdehaan

Reputation: 19928

Regarding the question itself I do not see how a segfault could occur from your code. I suspect the issue being in the code for Dlist, maybe a badly implemented destructor?

To solve your problem you could implement a count of elements inside Dlist and check for it. But maybe you are not allowed to modify Dlist. The preferred solution to avoid jumpy code and too much tests would be following suggestion. Instead of testing for the amount of operands just try it, and put the exception handler inside your processing method. Problem with this second solution is that the stack might remain in an inconsistent state: that means you cannot proceed with computations and should restart from the beginning.

void processInput(inputs usrInput, Dlist<double> &stack)
{
  try{
    // .... your old code WITHOUT ifs
  } catch(emptyList e){
    cout << "Not enough operands\n";
  }
}

I suppose the latter is a way to go, you could keep a copy of your stack and make the computation on one copy. The performance would be much better this way and above that the code easier to read and understand.

I hope it helps.

Upvotes: 1

djna
djna

Reputation: 55897

If you pass by value (not using reference) then you are effectively making a copy each time. Exactly how that is confusing things isn't clear, you'd need to study exactly what the various copy constructors are doing. But surely passing by reference is what you mean? Why would you copy a whole collection?

Upvotes: 0

Related Questions