Passing Pointers in Functions for Linked List

Question

#include <stdio.h>
#include <stdlib.h>
//why does this work with pointers thought they made a copy?
//am i freeing memory correctly and well?
//Something wrong with freeing
struct Node{

    struct Node* next;
    int data;

};
void newNode(struct Node* trans, int val)
{
    if(trans!=NULL)
    {
        while(trans->next!=NULL)
        {
            trans=trans->next;
        }
        //next is null create heap memory
        trans->next=malloc(sizeof(struct Node));
        //checking to see if memory is created
        if(trans->next==NULL)
        {
            printf("This has failed");
        }
        //put in data
        trans->next->data=val;      
        //next is null
        trans->next->next=NULL;
    }

}
void printList(struct Node* head)
{
    if(head!=NULL)
    {
        struct Node* current;
        current=head;
        while(current->next!=NULL)
        {
            //print that current nodes data
            printf("list is: %d\n",current->data);
            current=current->next;
        }
    printf("last element is: %d\n",current->data);

    }
    else
    {
        printf("list is empty!");
    }

}
int removeLastNode(struct Node* trans)
{

    //return -1 if its a empty list
    int val=-1;
    if(trans!=NULL)
    {
        /*have to access trans->next->next cause you are freeing trans->next->next and getting its val
        then you want to set tran->next to NULL!
        */
        while(trans->next->next!=NULL)
        {
            trans=trans->next;
        }
        //at end of the list?
        val=trans->next->data;
        //free the heap
        free(trans->next);
        //next points to null
        trans->next=NULL;

    }
    return val;
}
//LOOK AT ME!
void freeList(struct Node* root)
{
    struct Node* temp;
    struct Node* current;
    current=root;
    while(current->next!=NULL)
    {
        temp=current;
        //going to the next one
        current=current->next;
        //freeing previous
        free(temp);     
    }
    //Am I really freeing the last one?
    free(current);

    root->next=NULL;
    root=NULL;

}
void addingHundred(struct Node* trans)
{
    int i;
    for(i=0;i<100;i++)
    {
        newNode(trans,i);
    }
}

int main()
{
    struct Node* root;
    //create heap mem for root
    root=malloc(sizeof(struct Node));
    root->next=NULL;
    root->data=10;
    //traversal pointer
    struct Node* trans;
    //setting to point to root
    trans=root;
    //adding a new node..
    newNode(trans,8);
    printf("value of trans after function call: %p\n",trans);
    newNode(trans,12);
    //value does not change
    printf("value of trans after function call: %p\n",trans);
    addingHundred(trans);

    //printing the list
    printList(root);

    int storage;
    //removing last node
    storage=removeLastNode(trans);
    //returns the last nodes value
    printf("value removed: %d\n",storage);
    printList(root);
    freeList(root);
    printList(root);

    return 0;
}

I have a few questions about the code I have written above. The general conceptual question being in main I make a struct Node* tran with this struct I call the newNode function which takes in a struct Node*. Now I put in tran as a argument I do not pass the address of tran. In which case wouldn't the function newNode just create a copy of tran's value and any manipulation in the function would be undone after the function call?

I notice this with a print statement at least that tran's value does not change after the newNode function call. What I'm trying to get at is how is my linked list expanding, and being kept tracked of? Does passing tran's value as an argument work in this case because its pointing to the heap memory of the root's value initially, and then simply traversing the memory in heap, but not actually changing the contents of the memory?

If so then in order change the value of a node in the list I would have to pass &trans as an argument, but if I'm just traversing the list to add a node at the end I can pass tran as an argument?

My other question is I do not believe my freeList(struct Node* a) function is working correctly. As when I free root and then print it, it prints a garbage value for me when instead it should print "list is empty" or is it printing garbage cause its accessing memory I do not own?

Lastly, someone on here criticized my code for being "end user application code." I am still new to coding, and I was unsure if the code above is poorly formatted or what it means to be end user application code. I would be much appreciated if someone explained how I could avoid writing "end user application code."

the code should always check the returned value from malloc (and family) to assure the operation was successful — user3629249
– user3629249, Commented Jan 9, 2015 at 23:55
A copy of a pointer still points to the same place. So pointers can be passed as parameters and still work correctly. I.E. the place where the pointer points can be modified. However, it needs to be a pointer to a pointer to be able to change the place where the pointer points — user3629249
– user3629249, Commented Jan 9, 2015 at 23:58
@user3629249 Thank you, that was one of the things I needed clearing up on. To make sure traversing here works because we are not modifying where the pointer is pointing to at the end, but instead are using it to temporary go to the end of the list? — Nightlife
– Nightlife, Commented Jan 10, 2015 at 0:05

Tim B · Accepted Answer · 2015-12-04 16:57:50Z

You are not passing trans by value, you are passing a pointer to struct of Node, so a copy will never be made.

You say

I do not pass the address of tran

and that's absolutely wrong, what you are passing is in fact exactly that.

The value doesn't change because you are not modifying the pointer, after each call to newNode the trans pointer points exactly to the same address it did before the call, so no change in the value should be observed.

When you call newNode a new node is appended to the tail of the list, so if you actually traverse the list you would see all the values, the following code will print the values

struct Node *node;
for (node = root ; node != NULL ; node = node->next)
    printf("value %d @ %p\n", node->val, node);

there you should see the address of each node, and it's value

free is not meant to 0 the memory, instead you free it, to the operating system, it's like you give up ownership of the memory at that address, if there was an obligation to 0 the memory, there would be a huge performance hit due to that.

If you want to make the list, empty according to your code, you should do this exactly after calling freeList()

root = NULL;

I fixed your code

remark: your freeList function had a free(current) at the end which double frees the last node, so I removed it, also fixed some style stuff, and made the printList() function much more readable

#include <stdio.h>
#include <stdlib.h>
//why does this work with pointers thought they made a copy?
//am i freeing memory correctly and well?
//Something wrong with freeing
struct Node{

    struct Node* next;
    int data;

};
void newNode(struct Node* trans, int val)
{
    if (trans != NULL)
    {
        while (trans->next != NULL)
            trans = trans->next;
        /* next is null create heap memory */
        trans->next=malloc(sizeof(struct Node));
        /* checking to see if memory is created */
        if(trans->next == NULL)
            printf("This has failed");
        /* put in data */
        trans->next->data = val;
        /* next is null */
        trans->next->next = NULL;
    }
}
void printList(struct Node* head)
{
    struct Node *node;

    if (head == NULL)
        printf("empty list\n");
    for (node = head ; node != NULL ; node = node->next)
        printf("list is: %d\n", node->data);
}

int removeLastNode(struct Node* trans)
{
    int val = -1;
    /* return -1 if its a empty list */
    struct Node *node;
    struct Node *last;
    if (trans == NULL)
        return -1;
    /*
    * have to access trans->next->next cause you are freeing trans->next->next and getting its val
    * then you want to set tran->next to NULL!
    */
    node = trans;
    last = node->next;
    while (last->next != NULL)
    {
        node = node->next;
        last = node->next;
    }
    trans = node;
    node  = node->next;

    /* at end of the list? */
    val = node->data;
    /* free the heap */
    free(node);
    /* next points to null */
    trans->next = NULL;

    return val;
}

//LOOK AT ME!
void freeList(struct Node* root)
{
    struct Node* temp;
    struct Node* current;

    current = root;
    while (current != NULL)
    {
        temp=current;
        /* going to the next one */
        current=current->next;
        /* freeing previous */
        free(temp);
    }
}
void addingHundred(struct Node* trans)
{
    int i;
    for (i=0 ; i < 100 ; i++)
        newNode(trans, i);
}

int main()
{
    struct Node* root;
    int          storage;

    //create heap mem for root
    root = malloc(sizeof(struct Node));

    root->next=NULL;
    root->data=10;

    //adding a new node..
    newNode(root, 8);
    newNode(root, 12);

    addingHundred(root);

    //printing the list
    printList(root);


    //removing last node
    storage = removeLastNode(root);

    //returns the last nodes value
    printf("value removed: %d\n", storage);
    printList(root);

    freeList(root);

    root = NULL;

    printList(root);

    return 0;
}

I did this for your last comment, in your question body. Which I did not understand, but certainly you can improve your code formatting. Don't be afraid to use white space characters, the compiler ignores them anyway (except inside a string literal of course).

Collectives™ on Stack Overflow

Passing Pointers in Functions for Linked List

1 Answer 1

I fixed your code

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

I fixed your code

Comments

Your Answer

Sign up or log in

Post as a guest

Related