Segmentation fault class List

Question

I have the following class List defined. I'm trying to display all the elements through an override of [] operator that I have declared inside my class

template <typename Data>
class List{
protected:    
   ulong size = 0;

    struct Node{
        Data value;
        struct Node *next;

        //...
    }

    struct Node *top = nullptr;

public :
    //...
    Data& operator [](ulong) const;
    //...
};

//Specific Constructor
template <typename Data>
List<Data>::List(ulong newSize, Data value){
    size = newSize;
    struct Node* tmp;
    struct Node* tmp2 = nullptr;

    for(ulong i = 0; i < newSize; i++){
        tmp2 = new struct Node(value);
        if(top == nullptr){
            top = tmp2;
        }else{
            tmp = top;
            while(tmp->next != nullptr){
                tmp = tmp->next;
            }
            tmp->next = tmp2;
        }
    }
}

 //Copy Constructor
template <typename Data>
List<Data>::List(const List<Data>& ref){
    size = ref.size;
    struct Node* tmp = ref.top;
    struct Node* tmp2 = nullptr;
    struct Node* tmp3;

    while(tmp != nullptr){
        tmp2 = new struct Node();
        tmp2->value = tmp->value;
        if(top == nullptr){
            top = tmp2;
        }else{
            tmp3 = top;
            while(tmp3->next != nullptr){
                tmp3 = tmp3->next;
            }
            tmp3->next = tmp2;
        }
        tmp = tmp->next;
    } 
}

// Move Constructor
template <typename Data>
List<Data>::List(List<Data>&& ref){
    std::swap(size, ref.size);
    std::swap(top, ref.top);
}

template <typename Data>
Data& List<Data>::operator [](ulong i) const{
    if(size == 0)
        throw std::length_error("Lista vuota!");
    else if(i >= size)
        throw std::out_of_range("Lista troppo corta!");

    struct Node* tmp = top;
    for(ulong index = 0; index <= i-1; index++)
        tmp = tmp->next;

    return tmp->value;
}


int main(){
    List<int> l1(10,5); //List of 10 elements all equals to 5

    for(ulong i = 0; i < l1.getSize(); i++)
        std::cout << l1[i] << std::endl;

     return 0;
}

(I put the most important stuffs for the question) I didn't get any error during assignments ( ex: l1[3] = 20; ) but in this case I got a sigFault and I don't understand why. Thanks for the help.

Also `index <= i-1` is going to go badly since `index` and `i` are `ulong`, [see here](https://stackoverflow.com/questions/7221409/is-unsigned-integer-subtraction-defined-behavior) — Cory Kramer, Apr 08 '20 at 19:54
In `main()`, you wrote `for(ulong i < 0; i < l1.getSize(); i++)` did you mean `for(ulong i = 0; i < l1.getSize(); i++)` ? — melk, Apr 08 '20 at 20:10
Are you using a debugger? All of the hints suggested in the comments so far could have been easily discovered by utilizing a debugger. — PaulMcKenzie, Apr 08 '20 at 20:15
The `operator[]()` ASSUMES it is possible to traverse from `top` (i.e. `tmp = top` then doing `tmp = tmp->next` a total of `i` times, where `i` is between `0` and `size`). The behaviour is undefined if any of the `tmp->next` along the way are null or uninitialised. ALL the constructors need to ensure that is true when creating the object, and all other functions (including `operator=()`) that change the object needs to ensure it remains true. If you step through with a debugger, I'll bet you find cases where the copy constructor doesn't set things up correctly. — Peter, Apr 08 '20 at 20:16

score 0 · Answer 1 · answered Apr 08 '20 at 21:37

The for loop in operator[] is using an inappropriate condition. With an unsigned comparison of index <= i-1, and a value for i of 0, the condition is index <= unsigned(-1), which will always be true.

The condition could be rewritten as index < i:

for (ulong index = 0; index < i; index++)
    tmp = tmp->next;

Or your loop could be rewritten to be a while loop:

while (i-- > 0)
    tmp = tmp->next;

Segmentation fault class List

1 Answers1