Crash with circular linked list

Question

I'm trying to implemented a linked list (i believe this is a circular linked list) but upon using it, it eventually crashes. I think this is because the traversal pointer i use is not set properly to 0, and eventually dereferenced and exhibiting undefined behaviour.

I would appreciate your help.

This is the function i am using:

struct records t_head = 0;

struct records* find_id(Rect a)
{

    struct records* tmp;
    struct records* prev;

    // Add first node
    if (t_head == NULL) {
        tmp = new records;
        tmp->b = a;
        tmp->id = trecord_count + 1;
        tmp->tally.resize(labelsInfo.size());
        tmp->frames = 0;
        t_head = tmp;
        t_head->next = NULL;
        trecord_count++;
    }

    else {
        // Check if there is any node that has delete_flag set first
        tmp = t_head;
        while (tmp) {

            if (delete_flag) {

                // Delete here
                if (tmp == t_head) {
                    t_head = tmp->next;
                    delete (tmp);
                    trecord_count--;
                }
                else {
                    prev->next = tmp->next;
                    delete (tmp);
                    trecord_count--;
                }
            }

            prev = tmp;
            tmp = tmp->next;
        }

        tmp = new records;
        tmp->b = a;
        tmp->id = trecord_count + 1;
        tmp->tally.resize(labelsInfo.size());
        tmp->frames = 0;
        tmp->next = t_head;
        t_head = tmp;
        trecord_count++;
    }

    return tmp;
}

Struct definition is:

struct records {
int id;
Rect b;
vector<int> tally= {};
int frames=0;
int delete_flag=0;
struct records *next;
};

Answer 1

The following code runs but it also shows flaws beside of these already mentioned in the comments.

1. In your version you access the pointer in tmp which you just deleted in the loop for searching records with set delete_flag. This is corrected in the version below. The required modifications are marked with //< Tobias. Note, that you find such errors easily with valgrind under linux. Andon M. Coleman also mentions this in his comment.
2. In your version of find_id there is actually no search for a implemented. I've implemented such a search in my version.
3. If you delete some record in the middle of the list the end record keeps its id but you decrement trecord_count and assign the same id once again. So you end up with multiple records with the same id.

EDIT: I added some logging to the program. Maybe, that helps you to see what is going on. Furthermore, I misuse frames as a real id. I.e., no two records have the same frames-value. The records are in braces the full list is in square brackets. And NO you do not have a circular list. Maybe, you get the wrong impression because you are adding new records at the front and therefore, you need to use the old value of t_head as value for the next-pointer of the newly created record.

The output of the program is:

g++ -std=c++11 -g -O0 ./test.cc -o a.exe && ./a.exe
Returning { frames=1 id=1 b=1 }
Full content:
[ { frames=1 id=1 b=1 } ]

Adding { frames=2 id=2 b=2 }
Returning { frames=2 id=2 b=2 }
Full content:
[ { frames=2 id=2 b=2 }, { frames=1 id=1 b=1 } ]

Adding { frames=3 id=3 b=3 }
Returning { frames=3 id=3 b=3 }
Full content:
[ { frames=3 id=3 b=3 }, { frames=2 id=2 b=2 }, { frames=1 id=1 b=1 } ]

Deleting head { frames=3 id=3 b=3 }
Adding { frames=4 id=3 b=4 }
Returning { frames=4 id=3 b=4 }
Full content:
[ { frames=4 id=3 b=4 }, { frames=2 id=2 b=2 }, { frames=1 id=1 b=1 } ]

Returning { frames=2 id=2 b=2 }
Full content:
[ { frames=4 id=3 b=4 }, { frames=2 id=2 b=2 }, { frames=1 id=1 b=1 } ]

Deleting { frames=2 id=2 b=2 }
Adding { frames=5 id=3 b=5 }
Returning { frames=5 id=3 b=5 }
Full content:
[ { frames=5 id=3 b=5 }, { frames=4 id=3 b=4 }, { frames=1 id=1 b=1 } ]


At exit: [ { frames=5 id=3 b=5 }, { frames=4 id=3 b=4 }, { frames=1 id=1 b=1 } ]

There follows the program named test.cc:

#include <vector>
#include <iostream>
#include <cstdlib>

typedef int Rect;

struct records {
    int id;
    Rect b;
    std::vector<int> tally= {};
    int frames=0;
    int delete_flag=0;
    struct records *next;
};

struct records *t_head = 0;
int trecord_count = 0;
int trecord_frames = 0;

std::ostream& operator << (std::ostream& os, const records& r) {
    os << "{ frames=" << r.frames << " id=" << r.id << " b=" << r.b << " }";
    return os;
}

std::ostream& operator << (std::ostream& os, const records* r) {
    os << "[ ";
    if(r)
        os << *r;
    for(r=r->next; r; r=r->next)
        std::cout << ", " << *r;
    std::cout << " ]\n";
}

struct records* find_id(Rect a)
{

    struct records* tmp;
    struct records* prev;

    // Add first node
    if (t_head == NULL) {
        tmp = new records;
        tmp->b = a;
        tmp->id = trecord_count + 1;
        tmp->tally.resize(0);
        tmp->frames = ++trecord_frames;
        t_head = tmp;
        t_head->next = NULL;
        trecord_count++;
    }

    else {
        // Check if there is any node that has delete_flag set first
        tmp = t_head;
        while (tmp) {

            if (tmp->delete_flag) {

                // Delete here
                if (tmp == t_head) {
                    std::cout << "Deleting head " << *tmp << std::endl;
                    t_head = tmp->next;
                    delete (tmp);
                    trecord_count--;
                    tmp = t_head; //< Tobias
                }
                else {
                    std::cout << "Deleting " << *tmp << std::endl;
                    prev->next = tmp->next;
                    delete (tmp);
                    trecord_count--;
                    tmp = prev->next; //< Tobias
                }
            } else {
                prev = tmp;
                tmp = tmp->next;
            }
        }

        // Search for the id
        tmp = t_head;
        while(tmp && (tmp->b != a))
            tmp = tmp->next;

        if(!tmp) {
            tmp = new records;
            tmp->b = a;
            tmp->id = trecord_count + 1;
            tmp->tally.resize(0);
            tmp->frames = ++trecord_frames;
            tmp->next = t_head;
            t_head = tmp;
            std::cout << "Adding " << *tmp << std::endl;
            trecord_count++;
        }
    }

    std::cout << "Returning " << *tmp << std::endl;
    std::cout << "Full content:\n" << t_head << std::endl;

    return tmp;
}

int main() {
    find_id(1);
    find_id(2);
    find_id(3);

    t_head->delete_flag=1;
    find_id(4);

    records* r = find_id(2);
    r->delete_flag = 1;
    find_id(5);

    std::cout << "\nAt exit: " << t_head;

    return 0;
}

/*
    Local Variables:
    compile-command: "g++ -std=c++11 -g -O0 ./test.cc -o a.exe && ./a.exe"
    End:
*/