Help free structs and pointer.

Hey guys I'm having troubles understanding how to free a list of linked structs.

For simplicity reason we'll say that my struct has a value and a ptr to the next item. Here is waht I have but when I compile it, it doesn' t do anything.

Code:

---

void removePtr(LIST_ELEM *pHead, int loc){

        LIST_ELEM *ptr;
        LIST_ELEM *tmpPtr;
        int size = sizeOf(pHead);
        int i = 0;

        for(ptr = pHead; pHead != NULL; ptr++){
         
            if( loc == 0){
                tmpPtr = pHead;
                pHead = ptr->pNext;
                free(tmpPtr);
            }
            else if(loc = size-1){
                LIST_ELEM *ptrB4 = nodeB4(pHead,loc);
                ptrB4->pNext = NULL;
                free(ptr);
                }               
                               
            else if( i == loc){
                LIST_ELEM *ptrB4 = nodeB4(pHead,loc);
                tmpPtr = ptr;
                LIST_ELEM *ptrAfter = (tmpPtr->pNext);
                ptrB4->pNext = ptrAfter;
                free(tmpPtr);
                }
        i++;
        }
}

---

remove takes in the head of the list and the location/index where we want the item removed.

nodeB4 returns the item that come before the item we want to remove. Any ideas?

Quote:

---

Originally Posted by TaiL

Code:

---

void removePtr(LIST_ELEM *pHead, int loc){

        LIST_ELEM *ptr;
        LIST_ELEM *tmpPtr;
        int size = sizeOf(pHead);
        int i = 0;

        for(ptr = pHead; pHead != NULL; ptr++){
         
            if( loc == 0){
                tmpPtr = pHead;
                pHead = ptr->pNext;
                free(tmpPtr);
            } //if you pass in loc=0, won't you kill the entire list as you go through the loop?
            else if(loc = size-1){ //this is an assignment, not a comparison (also, why is it a special case?
                LIST_ELEM *ptrB4 = nodeB4(pHead,loc);
                ptrB4->pNext = NULL;
                free(ptr);
                }               
                               
            else if( i == loc){ //because of the assignment above, this never happens
                LIST_ELEM *ptrB4 = nodeB4(pHead,loc);
                tmpPtr = ptr;
                LIST_ELEM *ptrAfter = (tmpPtr->pNext);
                ptrB4->pNext = ptrAfter;
                free(tmpPtr);
                }
        i++;
        }
}

---

---

See comments in code.

Hm if its equal to 0 that would mean we want to remove the head of the "list" right? So then we'd have to change where the head of the list is which it then next item.


checking to see if its the last item of the list allows us to change the 2nd to last item of the list to point to null.

I think thats what I was going for.

I agree that the head is a special case (although the tail isn't). But you can't do this inside the loop -- otherwise you're going to delete the head of the list; and then you're going to delete the head of the list; and then you're going to delete the head of the list; and then....

But I just now noticed the reason your for-loop never happens in the first place: pHead != NULL is never true.

I actually took a big red marker and drew == above the function keys.

Nothing to do with the iching tho.

above the function keys?

wouldn't I need pHead!=Null so it could iderate through it all to find the item at the location?

Quote:

---

Originally Posted by TaiL

above the function keys?

wouldn't I need pHead!=Null so it could iderate through it all to find the item at the location?

---

Is pHead supposed to change through the loop?

HEY TABSTOP!! Sorry its suppose to be ptr not pHead. =)thnx

so after fixing that error. Here's what i have

Code:

---



void removePtr(LIST_ELEM *pHead, int loc){

        LIST_ELEM *ptr;
        LIST_ELEM *tmpPtr;
        int size = sizeOf(pHead);
        int i = 0;

        for(ptr = pHead; ptr != NULL; ptr++){
         
            if( loc == 0 && (size==0)){
                tmpPtr = pHead;
                pHead = ptr->pNext;
                free(tmpPtr);
                printf("The list is now empty.");
            }               
          else if( loc == 0){
                tmpPtr = pHead;
                pHead = ptr->pNext;
                free(tmpPtr);
            }       
            else if( i == loc){
                LIST_ELEM *ptrB4 = nodeB4(pHead,loc);
                tmpPtr = ptr;
                LIST_ELEM *ptrAfter = (tmpPtr->pNext);
                ptrB4->pNext = ptrAfter;
                free(tmpPtr);
                }
        i++;
        }
}

---

Checking the first two cases gives me a seg fault and checking the last error gives me a

Code:

---

glibc detected *** a.out: free(): invalid pointer:

---

errror.

going home so I'll be back tomorrow =(

Man I really didn't look at your for-loop. "ptr++" makes no sense in a linked list. Use "ptr=ptr->next" instead.

lol you know I don't know why i did that I originally had pNext instead of p++ =\
but for some reason it still doesn't seem to work giving me the same glibc error.

So it executes now but it doesn't free anything.

Code:

---


if( loc == 0 && (size==1)){
                tmpPtr = ptr;  // points tmpPtr to where Ptr is pointing
                pHead = NULL;//since its our only item we'll set pHead to point to null
                free(tmpPtr);// we then free tmpPtr
                printf("The list is now empty.");//lets usr know item was deleted.
               
            }

---

I don't know what you think size==1 is supposed to do, but size will always equal 4 (regardless of how many things there are on the list, et cetera).

sorry, size is equal to to a value returned by a method which returns the actual size of my list.

Okay, yes I see the capital O this time. So why do you say it doesn't free anything? If you mean the assigning of pHead to NULL isn't seen in the calling function, well, that's true. Changes made in the function stay in the function.

If you want to be able to change the value of the pointer, you need to pass in the address of the pointer (so that you would have a pointer-to-pointer-to-LIST_ELEM).

hm you kind of lost me there.
So I have this list LIST_ELEM which holds a bunch of structs

pHead <-strct1<-strct2

and when I remove it i want it to look like

pHead<-struct2

or in the first case

pHead<-strct1

remove returns

pHead


but i'm not getting anything like that, I'm getting my original list when print it out, or find the size

so if I call a remove on my list which holds 1 item, shouldn't the size of the list after the remove return 0?

So run the following code and play "spot the parallel":

Code:

---

void func1(int bob) {
    bob = 6;
}

void func2(int *bob) {
    bob = NULL;
}

int main(void) {
    int i = 11;
    int *iptr = &i;
    func1(i);
    printf("%d\n", i);
    func2(iptr);
    if (iptr == NULL) printf("Variable successfully removed.\n");
    printf("%p\n", iptr);
    return 0;
}

---

11 and an address is returned.

so i basically can't set something to null and say it was removed.

Even though you pass in a pointer to the function, that means you can only change the value of what is pointed to, not the value of the pointer itself (that's just a copy). So if you want to change the value of the pointer, which it seems like you do, you'll have to add an extra *.

So if:

LIST_ELEM *ptr = pHead;

and I want to free the actual item at *ptr

i would have to use

free(**ptr)? doesn't that return an invalid type error?

No you would have to use

Code:

---

void removePtr(LIST_ELEM **pHead, int loc)

---

and adjust accordingly.

Man that would make it more difficult for me.

so taking in a *pHead and then setting equal to say

Code:

---

LIST_ELEM **ptr = pHead

---

You could do that but you're back where you started -- pHead wouldn't change just because you changed ptr.

I'm starting to worry a little bit -- all the list functions require a elem** to work (creating, adding, deleting, ...)

EDIT: And for that matter, it's not as though the change is hard: do a search-and-destroy turning pHead to *pHead everywhere in the function and you're done!

LOL you're beginning to worry me now. I've been using (LIST_ELEM *pHead) for my add, print, and find methods.

Well, okay, print just takes one star. Find could take one * if you're not trying to change the list. But if you add something to the front of the list and try to change pHead = just_created_element, that won't work.

so my program is a fluke since I use one * for add, and sizeOf returns the right size after adding in an element.

Are you in a special case where you are always adding at the end or something?

always adding to the front

This I've got to see.

Code:

---


LIST_ELEM *add(LIST_ELEM *pHead,char type[],char name[], char title[], int year, double price, int size){

        LIST_ELEM *new_elem = malloc(sizeof(LIST_ELEM));

        strcpy(new_elem->type, type);
        strcpy(new_elem->name,name);
        strcpy(new_elem->title, title);

        new_elem->year = year;
        new_elem->price = price;
        new_elem->size = size;
        new_elem->pNext = pHead;

        return new_elem;

}

---

lol i think you helped me out on this the other day =)

Oh, you're returning the new head instead of changing the parameter.

Well ... why not do that here? Instead of returning void, return a LIST_ELEM* that's the new head of the list.

so I return pHead but that didn't solve my whole removal issue. Going back to the whole *ptr and **ptr deal, would I still have to do that?

HAH HAH finally getting somewhere. So I've got the case were the list is 1 element, and the element is the first element to work. But I can't get the last case to work, where the element is somewhere in the middle of the list. Can you spot where I'm going wrong? i get a seg fault. when i try this.

Code:

---


 else if( i == loc){
                LIST_ELEM *ptrB4 = nodeB4(pHead,loc);
                tmpPtr = ptr;
                LIST_ELEM *ptrAfter = (tmpPtr->pNext);
                ptrB4->pNext = ptrAfter;
                free(tmpPtr);
                break;
                }

---

Not really; you've got two choices:
Option 1: return void, change the parameter

Code:

---

void removePtr(LIST_ELEM **pHead, int loc) {
    LIST_ELEM *prevPtr, *nextPtr, *tempPtr;
    int size = sizeOf(*pHead);

    if (loc == 0) {
        if (size == 0) return; //already empty
        prevPtr = (*pHead)->next;
        free(*pHead);
        *pHead = prevPtr;
        return;
    }

    if (size < loc) return; //can't delete element that doesn't exist

    //if we're still here then walk to the element and delete it
    tempPtr = *pHead;
    for (int i = 0; i < loc; i++) {
        tempPtr = tempPtr->next;
    }
    prevPtr = nodeB4(*pHead, loc); //if we didn't have the function, we'd walk prevPtr with tempPtr
    nextPtr = tempPtr->next;
    prevPtr->next = nextPtr;
    free(tempPtr);
    return;
}

---

Option 2, return the head of the list (consistent with your add)

Code:

---

LIST_ELEM *removePtr(LIST_ELEM *pHead, int loc) {
    LIST_ELEM *prevPtr, *nextPtr, *tempPtr;
    int size = sizeOf(pHead);

    if (loc == 0) {
        if (size == 0) return NULL; //already empty
        prevPtr = pHead->next;
        free(pHead);
        pHead = prevPtr;
        return pHead;
    }

    if (size < loc) return pHead; //can't delete element that doesn't exist

    //if we're still here then walk to the element and delete it
    tempPtr = pHead;
    for (int i = 0; i < loc; i++) {
        tempPtr = tempPtr->next;
    }
    prevPtr = nodeB4(pHead, loc); //if we didn't have the function, we'd walk prevPtr with tempPtr
    nextPtr = tempPtr->next;
    prevPtr->next = nextPtr;
    free(tempPtr);
    return pHead;
}

---

I've assumed you still don't want loc==0 to kill the whole list. EDIT: Also I haven't tested it, or even compiled it, so caveat lector.

ah hah hah thnx again tabstop. Its workin now. (sigh) what a work out.