Thread: Stack with linked list, working, but I know I'm doing something dirty!

  1. #1
    Registered User
    Join Date
    Dec 2011
    Posts
    13

    Stack with linked list, working, but I know I'm doing something dirty!

    I finally got it working, but I'm not satisfied because I'm not freeing the memory myself. I'm not really sure what to do and was hoping to get a little advice.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    typedef struct list{
        char element;
        struct list * node;
    } list;
    
    int push(int data, list **ptr){
        list * tmp;
        tmp = malloc(sizeof(list));
        tmp->node = (*ptr)->node;
        tmp->element = data;
        (*ptr)->node = tmp;
        return 0;
    
    }
    
    int pop(list **ptr){
        if( (*ptr)->node != NULL ){
            int tmp;
            tmp = (*ptr)->node->element;
            free((*ptr)->node); //should be here I found out
            (*ptr)->node = (*ptr)->node->node;
            //free((*ptr)->node); causes junk data?
            return tmp;
        } else {
            fputs("Mismatched pop!\n",stderr);
            exit(1);
        }
    }
        
    int main(){
        list * head;
        head = malloc(sizeof(list));
        
        int i;
        for(i = 0;i<=10;i++)
            push(i,&head);
        for(i = 0;i<=10;i++)
            printf("%d\n",pop(&head));
        return 0;
    }
    Wow, I just figured it out, stupid mistake. Anyways, if anyone could at least check it to see if there are any other dirty tricks I pulled I'd be grateful.
    Last edited by XenoReseller; 04-12-2012 at 04:49 PM.

  2. #2
    Registered User
    Join Date
    Jan 2009
    Posts
    1,485
    Not sure what you did, but you could assign (*ptr)->node to a temporary pointer along with the data as you currently do. That way you don't lose track of it when you assign (*ptr)->node->node to it. Just before you return tmp, you free your temporary pointer (which has the old value of (*ptr)->node).

  3. #3
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Freeing memory doesn't automatically zero it out, which is why you seem to be getting away with it when you do this:
    Code:
            free((*ptr)->node); //should be here I found out
            (*ptr)->node = (*ptr)->node->node;
    The last line there could in all likelihood crash your program, because you're dereferencing memory you've just chucked out.


    Quzah.
    Hope is the first step on the road to disappointment.

  4. #4
    Registered User
    Join Date
    Dec 2011
    Posts
    13
    Would this fix it?

    Code:
    char pop(list **ptr){
        if( (*ptr)->node != NULL ){
            char tmp;
            list * temp;
            temp = malloc(sizeof(list));
            tmp = (*ptr)->node->element;
            temp->node = (*ptr)->node->node;
            free((*ptr)->node);
            (*ptr)->node = temp->node;
            free(temp);
            return tmp;
        } else {
            printf("\nMismatched pop!\n"); 
            exit(0); //Not a program error.
        }
    }

  5. #5
    Registered User
    Join Date
    Jan 2009
    Posts
    1,485
    No, and you have created another memory leak, why do you call malloc? Leaving out some code here but this is the basic idea.

    Code:
    list *old = (*ptr)->node;
    (*ptr)->node = (*ptr)->node->node;
    
    free(old);
    return tmp;

  6. #6
    Registered User
    Join Date
    Dec 2011
    Posts
    13
    Code:
    char pop(list **ptr){        if( (*ptr)->node != NULL ){                char tmp;                list * temp = (*ptr)->node;                tmp = (*ptr)->node->element;                (*ptr)->node = (*ptr)->node->node;                free(temp);                return tmp;        } else {                printf("\nMismatched pop!\n");                 exit(0); //Not a program error..        }}
    I'm not exactly sure how free(temp) is working, I think it frees what is pointed by the node pointed by the node of ptr...Right?

    Edit: sorry, I've tried 4 or 5 times to get it to be multi-line, but the computer I was on just wasn't cooperating.

    Instead:
    http://ideone.com/FlXGi

    Only the pop function is changed.
    Last edited by XenoReseller; 04-13-2012 at 08:19 AM.

  7. #7
    Registered User claudiu's Avatar
    Join Date
    Feb 2010
    Location
    London, United Kingdom
    Posts
    2,094
    free() works pretty much like this:

    Code:
    //some declarations and malloc
    int *jim = malloc(20 * sizeof(int));
    free(jim);
    /* jim is dead now and cannot be used */
    
    /*this is what you are doing */
    jim[5] = 3;
    1. Get rid of gets(). Never ever ever use it again. Replace it with fgets() and use that instead.
    2. Get rid of void main and replace it with int main(void) and return 0 at the end of the function.
    3. Get rid of conio.h and other antiquated DOS crap headers.
    4. Don't cast the return value of malloc, even if you always always always make sure that stdlib.h is included.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. stack using linked list problem
    By effa in forum C++ Programming
    Replies: 3
    Last Post: 09-18-2009, 12:10 PM
  2. STACK/FIFO as a linked list
    By BlackOps in forum C Programming
    Replies: 2
    Last Post: 07-24-2009, 02:04 PM
  3. Stack in linked list
    By mag_chan in forum C Programming
    Replies: 2
    Last Post: 11-09-2005, 05:31 AM
  4. Linked list Stack question
    By lyrick in forum C++ Programming
    Replies: 4
    Last Post: 09-23-2005, 06:23 AM
  5. linked list stack question
    By Drew in forum C++ Programming
    Replies: 2
    Last Post: 09-11-2003, 05:05 AM