CODEWITHSUNDEEP
clinked-list
livelaughlove
livelaughlove

Reputation: 386

Linkedlist not looping properly

I have a linked list of particles. I would like to make these particles move one-by-one. So in order to do that I need to loop through every particle in my linked list, and when it reaches the last particle, I would like it to go back to the first particle. but my program is not doing that.

int particle_update(struct particle **head ){
    struct particle *current = *head;
    struct particle *next;
    printf("particle_update\n");

    while(current != NULL){
        while(current != NULL && current->lifespan >=0){
            current->lifespan --;

            current->pos.y = current->pos.y + (current->spd.y * current->dir.y);
            current->pos.x = current->pos.x + (current->spd.x * current->dir.x);
            current->pos.z = current->pos.z + (current->spd.z * current->dir.z);

            current = current->next;
            if (current == NULL)
                current = *head;
        }
    }

    particle_destroy(head);
    return 0;
}

Upvotes: 0

Views: 146

Answers (2)

rsj
rsj

Reputation: 788

I suspect what is going on here is that to destroy the particle you need to modify the particle before it, and improper handling of this case is what is tripping you up.

First, as soon as you hit the last node in your linked list, you call current = current -> next at the beginning of your while loop whereas you should be calling it at the end.

As a result, current is now null so you are going to hit BAD_EXEC errors when you call current->position as you are de-referencing a null pointer. Instead, increment current at the end of the while loop so that you never de-reference a null pointer.

Next, pointing current to head means that you never get to exit your loop except via particles expiring, which I assume is not what you want (otherwise you would have a while(1)).

So here is a better alternative for handling the surgery:

int particle_update(struct particle **head ){

    struct particle * current = *head;
    struct particle * prev = NULL;

    while (current != NULL) {
        // lifespan check
        current->lifespan = (current -> lifespan > 0) ? current->lifespan-1:particle_destroy(&prev, &current);

        // update position of current 
        ...


        // increment counter at end of while loop
        prev = current;
        current = current -> next;  //now current is always one node ahead of previous.
    }

    return 0;
}

Then, your particle destroy function would be:

void particle_destroy(struct particle ** prev, struct particle ** current) {
    if (*current = NULL) return;  //nothing to do

    struct particle * tmp = *current;
    if (*prev != NULL)  { /* need to modify previous node */
        (*prev) -> next = current -> next;
    } else { /* head has expired, so change head ptr to next node */
        (*current) = (*current) -> next;
    }

    /* free resources */

    // do other clean-up, if necessary.
    free(tmp);

    return;
}

Upvotes: 0

Keith Nicholas
Keith Nicholas

Reputation: 44298

I got a feeling there's a number of problems....

one.... this is strange...

while(current->lifespan >= 0 && current != NULL){

it should be while(current != NULL && current->lifespan >= 0){

this means it will check its not null first, and only if it is not null, it will try and see what current->lifespan is. The way you have it, it will likely crash

also, I'm not sure if you want to move to the next as the first thing? I think it might be the last thing you want to do inthe loop

also, the outer loop will loop forever once you get the inner loop doing what you want.

Upvotes: 1

Related Questions