I can't understant why I can't add a node at the end like this

Question

I am trying to insert a new node at the end of the list. I know the first way is the "right way". But I am trying another way with another function (2nd function) like this but it seems there are not changes to my list, any thoughts?

typedef struct listnode *listptr;

struct listnode {
    int value;
    listptr next;
};

void insert_at_end(listptr *x, int n) {
    while ((*x) != NULL) {       
        x = &((*x)->next);   
    }                       
    *x = (listptr)malloc(sizeof(struct listnode));  
    (*x)->next = NULL;   
    (*x)->value = n;    
}

void insert_at_end_2(listptr x, int n) {
    listptr newnode = (struct listnode *)malloc(sizeof(struct listnode));
    newnode->next = NULL;
    newnode->value = n;

    while (x != NULL) {
        x = x->next;
    }
    x = newnode;
}

`x` is a local variable. Changing it has no effect on the caller's variable. — kaylum, Dec 24 '19 at 00:15
Does this answer your question? [How to change value of variable passed as argument?](https://stackoverflow.com/questions/9459691/how-to-change-value-of-variable-passed-as-argument) — kaylum, Dec 24 '19 at 00:17
Fyi, In `insert_at_end_2`, `malloc(sizeof(listptr))` is wrong. It should be `malloc(sizeof *x)`, or `malloc(sizeof(struct listnode))`. And that, kids, is why hiding pointer types in typedef aliases is a terrible idea. Perchance, what does that function actually *return*? You neglected that rather important piece of info. Unrelated, [don't cast malloc in C progams](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — WhozCraig, Dec 24 '19 at 00:19
i edited what you noticed the original code was like you said — average Joe, Dec 24 '19 at 00:26
@averageJoe Great, now, look at Kaylums comments. It's the difference between the first and second version. — WhozCraig, Dec 24 '19 at 00:36
@ WhozCraig i cant totally understand what he says.i understant that it has no effect on the callers variable but x points to memory location.That doesnt mean change to tha location i make changes?? — average Joe, Dec 24 '19 at 00:44
`x` points to memory location so you can change the contents of what it points to by dereferencing the pointer. However you can't change the original pointer value itself. — kaylum, Dec 24 '19 at 00:58
You will want to review: [Is it a good idea to **typedef** pointers?](http://stackoverflow.com/questions/750178/is-it-a-good-idea-to-typedef-pointers). — David C. Rankin, Dec 24 '19 at 08:00

score 0 · Answer 1 · answered Dec 24 '19 at 01:31

There are two problems with this function implementation.

The first one is that the function deals with a copy of the original node passed as an argument to the function. So changing the copy does not have effect on the original argument.

The second problem is that after this loop

while (x!=NULL){
    x = x->next;
}

x will be equal to NULL. So the next statement

x =newnode;

does not change the data member next of the last node. So the list will be unchanged.

Using the approach when the head node is not passed by reference the function implementation can look the following way.

listptr insert_at_end_2( listptr x, int n )
{
    listptr newnode = malloc( sizeof( *listptr ) );

    if ( newnode != NULL )
    {
        newnode->next = NULL;
        newnode->value = n;

        if ( x == NULL )
        {
            x = newnode;
        }
        else
        {
            listptr current = x;

            while ( current->next != NULL )
            {
                current = current->next;
            }
            current->next  = newnode;
        }
    }

    return x;
}

However there is one drawback with this implementation like and with the first implementation when the head node is passed by reference: the function does not report whether a new node was allocated successfully.

So a better implementation of the function can look like

int insert_at_end( listptr *x, int n )
{
    listptr newnode = malloc( sizeof( *listptr ) );

    int success = newnode != NULL;

    if ( success )
    {
        newnode->next  = NULL;   
        newnode->value = n;

        while ( *x != NULL )
        {       
            x = &( *x )->next;   
        }                       

        *x = newnode;
    }

    return success;  
}

score 0 · Answer 2 · answered Dec 24 '19 at 10:52

@Vlad from Moscow i made your code similar to mine.So this works.


void insert_at_end_2( listptr x, int n )
{
  listptr newnode =  (listptr)malloc( sizeof( listptr ) );
  newnode->next = NULL;
  newnode->value = n;


  while ( x->next!= NULL )
  {
      x = x->next;
  }
  x->next  = newnode;

}

I can't understant why I can't add a node at the end like this

2 Answers2