Removal item in threaded list C

2

Well, I'm having a little problem, I'm implementing a simple A *. When I pass the item already checked to the closed list and do the deallocation of the open list item, it is giving undeclared pointer error ... The error is this pointer being freed was not allocated , but I already checked my logic and also checked if I I was handing out a pointer that actually had been declared with malloc and it really was! I even use the same code, another code deallocation code. I practically copied and pasted each other and what was already done and does not give problems, but in the new gives ... See:

This is the deallocation code that is working (I use this code to animate too):

void anima(Lista* li, int frame, int agFramex, int agFramey){
    Elem* no = (Elem*) malloc(sizeof(Elem));
    Elem* ant = (Elem*) malloc(sizeof(Elem));
    no = (*li);
    ant = no;
    while(no != NULL){
        if(no->dados.y > ALT+200){
            if(no == *li){
                *li = no->Prox;
                free(no);
                no = *li;
            } else if(no->Prox == NULL){
                ant->Prox = no->Prox;
                free(no);
                no = ant->Prox;
            } else {
                ant->Prox = no->Prox;
                free(no);
                no = ant->Prox;
            }
        } else {
            no->dados.y += no->dados.vel;
            al_draw_bitmap_region(no->dados.Img, agFramex*frame, agFramey*frame, frame, frame, no->dados.x, no->dados.y, 0);
            ant = no;
            no = no->Prox;
        }
    }
}

This is the code that is not: ( atual and AbList exist, they are as function parameter up there, since the function was very extensive I chose not to put it whole here, in the previous moments of the function eu I put elements in the list just that):

Elc* rem = (Elc*) malloc(sizeof(Elc));
Elc* mer = (Elc*) malloc(sizeof(Elc));

rem = (*AbList);
mer = rem;
while(rem != NULL){
    if(rem->dados.x == atual.dados.x && rem->dados.y == atual.dados.y){
        if(rem == *AbList){
            *AbList = rem->Prox;
            free(rem);
            rem = *AbList;
        } else if(rem->Prox == NULL){
            mer->Prox = rem->Prox;
            free(rem);
            rem = mer->Prox;
        } else {
            mer->Prox = rem->Prox;
            free(rem);
            rem = mer->Prox;
        }
    } else {
        mer = rem;
        rem = rem->Prox;
    }
}

As you can see, the codes are very similar, but for some reason in this I get the error and in the other not ...

Note: I use Macbook Pro I am coding in C using Allegro.

    
asked by anonymous 25.10.2015 / 04:48

1 answer

2

I assume that Lista is Elem* and that Lista* is Elem** . Correct?

Let's see the beginning of the first function:

Elem* no = (Elem*) malloc(sizeof(Elem));  // Aloca um elemento
Elem* ant = (Elem*) malloc(sizeof(Elem)); // Aloca outro elemento
no = (*li);                               // Perde a referência do no alocado. Memory leak.
ant = no;                                 // Perde a outra referência também. Memory leak.
// Neste ponto no == ant == *li

Now the second:

Elc* rem = (Elc*) malloc(sizeof(Elc)); // Aloca um elemento.
Elc* mer = (Elc*) malloc(sizeof(Elc)); // Aloca um elemento.

rem = (*AbList);                       // Perde a referência. Memory leak.
mer = rem;                             // Perde outra referência. Memory leak.
// Neste ponto mer == rem == *AbList

Let's look at more of your first function:

            if(no == *li){
                *li = no->Prox;
                free(no);
                no = *li;
                // Esquece que ant pode ser igual a no.
                // Se for igual, agora ant é um ponteiro inválido!
                // Se for diferente, então ant->Prox é um ponteiro inválido!

And also:

            } else if(no->Prox == NULL){
                ant->Prox = no->Prox; // Ou seja, ant->Prox = NULL;
                free(no);
                no = ant->Prox; // Ou seja, no = NULL;
            } else {
                // Este bloco é idêntico ao anterior!
                ant->Prox = no->Prox;
                free(no);
                no = ant->Prox;
            }

So, never let ant point to anything other than the previous node of no . And if no is the first, that ant is NULL . Or he has to point to something valid or NULL , never leave him pointing to something invalid! The same can be stated on ant->Prox .

And also, all code in } else if (x) { A; } else { A; } , where evaluation of x has no side effects, can be replaced simply by } else { A; } .

void anima(Lista* li, int frame, int agFramex, int agFramey){
    Elem *no = *li;
    Elem *ant = NULL;
    while (no != NULL) {
        if (no->dados.y > ALT + 200) {
            if (no == *li) {
                *li = no->Prox;
                free(no);
                no = *li;
                // Aqui ant era NULL e continua sendo NULL.
            } else {
                // Se entrou aqui, então ant não é NULL.
                // Isso funciona mesmo se no->Prox for NULL.
                ant->Prox = no->Prox;
                free(no);
                no = ant->Prox;
            }
        } else {
            no->dados.y += no->dados.vel;
            al_draw_bitmap_region(no->dados.Img, agFramex * frame, agFramey * frame, frame, frame, no->dados.x, no->dados.y, 0);
            ant = no;
            no = no->Prox;
        }
    }
}

What really changes the behavior in this code over the previous one is just that the memory leaks were deleted and that ant = no; of the beginning was replaced with Elem *ant = NULL; . >

We can apply the same transformations to your second function, which has the same structure. Here's the result:

Elc* rem = *AbList;
Elc* mer = NULL;

while(rem != NULL){
    if (rem->dados.x == atual.dados.x && rem->dados.y == atual.dados.y) {
        if (rem == *AbList) {
            *AbList = rem->Prox;
            free(rem);
            rem = *AbList;
            // Aqui mer era NULL e continua sendo NULL.
        } else {
            // Se entrou aqui, então mer não é NULL.
            // Isso funciona mesmo se rem->Prox for NULL.
            mer->Prox = rem->Prox;
            free(rem);
            rem = mer->Prox;
        }
    } else {
        mer = rem;
        rem = rem->Prox;
    }
}

Oh yes, I almost forgot. If the atual address is something that may appear in the list started by *AbList , this second code will always hit if you get a case where rem == &atual and free(rem); is run and then you try to access atual.dados.x or atual.dados.y . If this happens, make a copy somewhere else that is off the list and only access the copy. If that does not happen (or if atual is already exactly that copy), then everything is quiet about it.

    
25.10.2015 / 05:44