CODEWITHSUNDEEP
c++pointersg++segmentation-fault
Josh
Josh

Reputation: 295

What is the source of this segmentation fault?

I've got a bizarre error here: 882 Segmentation fault ./a.out

The segment of code:

int end=array.Length, loop=0;
cout<<end<<" about to print";
for(;loop<end;loop++){
    cout<<"\nMr. loop says: ";
    array.get(loop).print(fout);
}

Output of my program:

enpty initializer called for llist
enpty initializer called for entry
adding
adding
999deleting999
done deleting
123deleting123
done deleting
333deleting333
done deleting
printing
2 about to print
./g+: line 7:   882 Segmentation fault      ./a.out

and vital output of:

printing
2 about to print
./g+: line 7:   882 Segmentation fault      ./a.out

meaning that the error is this line:

for(;loop<end;loop++){

which has proven good values and is syntactically correct (yes I know its bad style, though).

Ideas? No one at my university seems to be able to help me with this.

here are the files:

Upvotes: 1

Views: 252

Answers (4)

phihag
phihag

Reputation: 288260

Unless you're doing low-level memory operations (and you're not doing this in the posted program), a segmentation fault is a sign of memory corruption, i.e. you've messed up some coding. Note that the memory corruption typically occurs before the error is triggered. In extreme cases, initial memory corruption and actual segmentation fault can be hours and modules apart.

Your first step should be to run the program in valgrind or gdb to find out the details of the segmentation fault. Also, always compile with at least gcc -Wall and take note of every warning - unless you're modifying the compiler, changes are every single warning indicates a bug.

In your case, the error is almost certainly in the implementation of llist. There is a number of problems with that implementation:

  • Implementations don't belong in .h files, they should be in .cpp files.
  • entry.towardsback and entry.towardsfront are usually called prev(ious) and next - those names are shorter and easier to distinguish.
  • hold should probably be called head, in accordance with the general naming conventions of linked lists.
  • A list of length 0 shouldn't have any entries, i.e. be initialized with hold == NULL.
  • From line 61 (in add):

hold->towardsback = node;
hold->towardsback->towardshead = node; // node->towardshead = node;

The second line is almost certainly wrong. You probably want to configure node first and then just set hold->towardsback = node;.

Upvotes: 3

Moo-Juice
Moo-Juice

Reputation: 38820

Ok, I see several issues with a cursory glance over your code.

  1. In your entry<> class

    operator T* () {return &stud;}

stud is already a T*. You are now returning the address of that pointer, rather than the pointer itself.

  1. In your llist class

    if(temp==temp->towardsback){
        delete this;

Which will, in effect, delete your list class if this condition is true. Ouch?

Personally, I think you need to start again on your list implementation :)

Upvotes: 1

littleadv
littleadv

Reputation: 20282

Its most likely because how you manage your list. You're adding/removing dynamically allocated objects, and pass them by reference. It's a bad idea.

Upvotes: 1

Daniel
Daniel

Reputation: 11

I work mostly with c#, but are arrays 0 index in c++? So, you would need to set end to array.Length-1. At least in c#, the first element in the array would be array[0].

Upvotes: 0

Related Questions