If I understand the static analyser's reasoning correctly:

while (edges && edges->delete) { // the check of 'edges' implies it potentially can be NULL
        e = edges;
        DELETE(edges, e);
    }
    e = edges->next;  // if edges IS in fact null, this will cause crash
...

If my understanding of the above is right, it should probably be enough to add a if (!edges) return; before e = edges->next;.

If my understanding of the above is right, it should probably be enough to add a if (!edges) return; before e = edges->next;.

i guess using if (edges) will work the same
because for !edges using return doesn't look good because function is void it may lead to additional warning in cppcheck i think.

If my understanding of the above is right, it should probably be enough to add a if (!edges) return; before e = edges->next;.

i guess using if (edges) will work the same because for !edges using return doesn't look good because function is void it may lead to additional warning in cppcheck i think.

In effect they are the same. return; is perfectly valid and only way to return early in a void function (note: there is no return value). Let's compare them side by side:

    if (edges) {
        e = edges->next;
        do {
            if (e->delete) {
                t = e;
                e = e->next;
                DELETE(edges, t);
            }
            else
                e = e->next;
        } while (e != edges);
    }

    if (!edges)
        return;

    e = edges->next;
    do {
        if (e->delete) {
            t = e;
            e = e->next;
            DELETE(edges, t);
        }
        else
            e = e->next;
    } while (e != edges);

In my opinion the readability and intent of the code is much better with the latter (tip: search for "early return" pattern and "pyramid of doom" anti-pattern).

To the root cause of this issue. I managed to reproduce this with Clang's scan-build¹, and it became more clear whence it originates. The macro DELETE may set edges to NULL if edges == edges->next:

We have the same problem with CleanFaces() and a similar one with CleanVertices() too.

@metzm I'm not quite sure of the implications if edges == edges->next and we exit the function early. Was everything that were supposed to be cleaned up, actually cleaned up to that point?

We have the same problem with CleanFaces() and a similar one with CleanVertices() too.
indeed

@metzm I'm not quite sure of the implications if edges == edges->next and we exit the function early. Was everything that were supposed to be cleaned up, actually cleaned up to that point?

I think exiting the function early with

if (!edges)
        return;

is safe because there would be no edges left. edges is the root pointer to the double linked list, if it is NULL, the list is empty.

@nilason Or others, if you are ok with Markus Metz's response and approval, let's merge this. On Monday I'll get it merged if everything is still fine

Let us address CID: 1207410 and CID: 1207411 too, with this PR.

@metzm Thank you very much for your input!

I did the changes.
also as you suggested @nilason i did the early return. I agree with you to exit early