2

I am new to programming.The other day I was playing around with structures and pointers...I was getting errors.I tried to rectify them.It got rectified...But I cant justify why there was an error at the first place.Please help me...

struct node{
   int data;
   struct node*next;    
};  

int main(){
   struct node *newnode=NULL;
   struct node *start=NULL;
   newnode=start=(struct node*)malloc(sizeof(struct node));
   newnode->data=1;

   //code snippet 
   newnode->next=NULL;
   newnode=newnode->next;
   newnode=(struct node*)malloc(sizeof(struct node));
   newnode->data=2;
   start=start->next;//error probably as start->next is perceived as NULL Address
   printf("%d",start->data);
   return 0;    
}

when replacing code snippet with this code

newnode->next=(struct node*)malloc(sizeof(struct node));
newnode=newnode->next;
newnode->data=2;
start=start->next;
printf("%d",start->data);

error dissapers..How does one justify this ?

Improve this question
3
  • 1
    Don't cast the return of malloc Commented May 25, 2018 at 19:45
  • Note that it is unnecessary (but not harmful) to initialize your pointers to NULL when the very next thing you do is assign different values to them. Commented May 25, 2018 at 19:49
  • As John says, just directly initialize your pointers if that's literally the next thing you were going to do anyway. Keeps your code more minimal and looks more consistent. Commented May 25, 2018 at 20:48

5 Answers 5

Reset to default
2

You are overwritting the address of newnode here

newnode = newnode->next;

You probably want:

start = malloc(sizeof(struct node));
start->data = 1;

newnode = malloc(sizeof(struct node));
newnode->data = 2;
newnode->next = NULL;

start->next = newnode;

printf("%d", start->data);
Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

He knows how to fix it, his second version did that. He wanted to know why the first version was wrong.
@Barmar, well, in the second version OP is doing the same error: start=start->next; (overwrites the value)
But it doesn't cause a problem because start->next points to a valid node. In the first version start->next was a null pointer.
@Barmar, I don't think so: newnode=start=(struct node*)malloc(sizeof(struct node)); , the result of malloc is assigned to both pointers, anyway, it is not clear to me what OP is trying to do.
But then he does newnode->next=(struct node*)malloc(sizeof(struct node));.
|
1

In the first code, newnode and start both start off pointing to the same node that you allocated. You then set newnode->next = NULL;, so that also sets start->next to NULL.

You then do:

newnode = newnode->next;
newnode = (struct node*)malloc(sizeof(struct node));

so newnode now points to a different node, but start still points to the original node. So start->next is still NULL. Then you do:

start = start->next;

This sets start to NULL, so trying to print start->data is invalid because you can't dereference a null pointer.

The first assignment before malloc() is pointless, because you're immediately replacing the variable with something else. I'm guessing you thought that by first reassigning newnode, the malloc() call would update both newnode and newnode->next because you've declared them to be equivalent. But that's not how assignments work -- all it does is copy the value of newnode->next into newnode, it doesn't link those two places together.

Your second version gets this right by assigning to newnode->next rather to newnode. This also assigns to start->next because newnode and start initially point to the same structure. Then you assign

newnode = newnode->next;

This updates newnode, but start is still OK and points to the first node. When you then do

start = start->next;

it updates start to point to the second node as well. In this case, start->data is valid, it contains the same thing that you assigned to newnode->data a few lines earlier.

Improve this answer

Comments

0

You are correct that the error comes from printf("%d",start->data); where you are trying to dereference a pointer to NULL.

Let me explain it graphically: with the firs 4 lines you get this:

             _____________
newnode ->  | data = 1    |
start   ->  | next = null |
            |_____________|

This line newnode->next=NULL; does nothing. Then, newnode=newnode->next; does this:

             _____________
            | data = 1    |
start   ->  | next = null |      newnode -> NULL
            |_____________|

Then, 

 newnode=(struct node*)malloc(sizeof(struct node));
 newnode->data=2;`

does this:

             _____________                    _____________
            | data = 1    |                  | data = 2    |
start   ->  | next = null |      newnode ->  | next = null | 
            |_____________|                  |_____________|

Finally, start=start->next; sets the state like this, and clearly you try to access a field of a pointer to NULL as a result.

                                              _____________
                                             | data = 2    |
start   ->  null                 newnode ->  | next = null | 
                                             |_____________|
Improve this answer

1 Comment

Ok i kind of get in now..Thank you for the answer..Was quite Helpful!!!
0

In the first of your two codes, you have

newnode=newnode->next;
newnode=(struct node*)malloc(sizeof(struct node));

. The first of those assignments is useless: whatever value is thereby assigned to newnode is in the next line replaced by a different one. Moreover, nothing is assigned to the next member of the structure to which newnode initially pointed. The first assignment to newnode does not somehow bind it to that next pointer, as an alias for the pointer itself. It just makes newnode point to the same thing.

In the second code, you instead have:

newnode->next=(struct node*)malloc(sizeof(struct node));
newnode=newnode->next;

That makes a lot more sense. You first allocate memory and assign newnode->next to point to it, then you update newnode to point to the new memory, too. Furthermore, up to that point you still have start pointing to the first dynamically-allocated block, so you can still access that, and since newnode and start initially pointed to the same structure, the same value can afterward be obtained via the expression start->next, too.

Improve this answer

Comments

0

Let me comment what the first version does:

newnode=start=(struct node*)malloc(sizeof(struct node));
...
// at this point, newnode and start point to the same node

newnode->next=NULL; 
// since start points to the same node as newnode, this means that also 
// start->next will be NULL 
...
start=start->next; // since start->next is null, start will be null after this statement

printf("%d",start->data);  // start is null, so start->null dereferences NULL (undefined behaviour, e.g. a crash or something dubious)

Your second version is different, because...

newnode->next=(struct node*)malloc(sizeof(struct node));
// this lets newnode->next point to a valid (new) node
// since start==newnode, also start->next will point to this valid (new) node

start=start->next;  // afterwards, start will point to the valid (new) node
printf("%d",start->data); // OK; should print 2 then.

I think you might confuse an assignment expression with "defining an alias":

newnode=newnode->next;
newnode=(struct node*)malloc(sizeof(struct node));

is very different from

newnode->next = (struct node*)malloc(sizeof(struct node));

because

newnode = newnode->next

is not defining a macro or a replacement text for newnode such that text newnode is replaced with text newnode->next henceforth. It's rather that the variable newnode will get a new value, i.e. the value that member next of this node points to.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.