coping data to a node value in c gives segmentation fault but malloc removes it

Question

I have the below program for inserting node in bst

#include <stdio.h>
#include <stdlib.h>


struct node {
    int key;
    struct node * left;
    struct node * right;
};

struct node * insert(struct node *,int);

int main( int argc, const char *argv[]){
    struct node *root= malloc(sizeof(struct node ));

    int x;

    if(root==NULL){
        printf("mem error"); 
        return;
    }
    root=NULL;


    while(1){
        fprintf(stdout, "Enter the value of data\n" );
        fscanf(stdin,"%d",&x);
        root=insert(root,x);

    }


}

struct node * insert(struct node * root, int data ){

    if(root==NULL){
       struct node *root= malloc(sizeof(struct node ));
       root->key = data;
       printf("hiii\n");
       root->left = NULL;
       root->right = NULL;
       return root;
    }else{
       if(root->key >= data){
          root->left = insert(root->left,data);
       }else if(root->key <= data){
          root->right = insert(root->right,data);
       }
       return root;
    }
}

it runs fine.. but if I comment the malloc line in insert function..it gives me a segmentation fault after taking the first value. what is going on here??

Why does it surprise you that disabling the `malloc` in `insert` gives a segmentation fault? What did you expect to happen? — zwol, Jul 26 '13 at 16:20
Learn a bit about formatting code when you post a question here. I fixed it for you. You can look at what I did by clicking your 'edit' button. Basically I removed all the blank lines that nobody wants to see and indented everything 4 spaces by clicking the little `{}` icon. You will get better answers with a better question. — Lee Meador, Jul 26 '13 at 16:23

simonc · Answer 1 · 2013-07-26T16:55:46.370

1

If you comment out line that allocates memory for root, you're left with

if(root==NULL) {
    //struct node *root= malloc(sizeof(struct node ));
    root->key = data;

which dereferences a NULL pointer. This results in undefined behaviour. Anything can happen at this point. A crash would be most useful and is very likely but isn't guaranteed. If your question is saying that you manage to write to root->key, this is possible but don't count on it working on different builds or different hosts (or even with different runs of the same program on the same computer).

edited Jul 26 '13 at 16:55

answered Jul 26 '13 at 16:20

simonc

41,632
12
85
103

@Jim The question says "but if I comment the malloc line in insert function..it gives me a segmentation fault". I took that to mean... well, what I said in my answer. How do you interpret the statement? – simonc Jul 26 '13 at 16:29
1

@Jim No the answer is correct, read title of question..OP wants why using malloc() remove segmentation fault. – Grijesh Chauhan Jul 26 '13 at 16:36
Look at the "hiii" print. He's obviously puzzled why it didn't crash right away. Otherwise it would be a totally trivial question. – Jiminion Jul 26 '13 at 16:39
@Jim are you talking about *`but if I comment the malloc line in insert function..it gives me a segmentation fault after taking the first value. what is going on here?`* – Grijesh Chauhan Jul 26 '13 at 16:49
Yes. I am giving the OP the benefit of the doubt. – Jiminion Jul 26 '13 at 16:54
Ok, I've updated my answer with a note of why undefined behaviour can include sometimes being able to dereference uninitialised (or otherwise bad) pointers – simonc Jul 26 '13 at 16:56

Grijesh Chauhan · Accepted Answer · 2013-07-26T17:06:39.907

You can't comment malloc-line in insert function. The use of malloc() is for allocate new memory dynamically for new nodes to insert in linked-list.

Read: void* malloc (size_t size);

Allocate memory block
Allocates a block of size bytes of memory, returning a pointer to the beginning of the block.

So suppose if you comment/remove malloc line in insert() (read comment):

//struct node *root= malloc(sizeof(struct node ));
root->key = data;  // here illegal operation

root->left = NULL;  //   here illegal operation
root->right = NULL;  //  here illegal operation

then root value will be garbage and you are access and perform read/write action on garbage address not allocated by the process - an memory illegal operation.

Your code will compile because syntax-wise the code is correct but at runtime the OS will detect illegal memory access and can terminate your code. (interesting to note: as OS detects memory right violation by a process -- An invalid access to valid memory gives: SIGSEGV And access to an invalid address gives: SIGBUS). --Actually this cause Undefined behavior. In the worst case your program may seem execute without any failure, producing garbage results.

it gives me a segmentation fault after taking the first value. what is going on here?

As I explained above removing malloc() makes your program behaving in Undefined manner(read: Undefined behaviour). If program runs as Undefined behavior you can's guess what can be happen, in worst case your program may execute without any failure, or may be run partial (that is what you are observing). Thing is that when OS detects illegal memory access and send a signal to terminate the process. (may be first five random-garbage value to root points to a memory associated with our program and according to OS access to that memory is not a illegal operator - while for fifth time random value of root is a memory that doesn't belong to the process and read/write operation on that memory detected by OS hence your code finally terminate with segmentation fault).

..Thanks for the answer...My doubt is this.. I have already malloc a variable root in the main function. Now that same variable is the first parameter to the function insert()..So the node variable in insert() should have the same value right? Why the need of malloc again?? — Pranab Boruah, Jul 26 '13 at 19:17
@PranabBoruah as I explained in first line *`The use of malloc() is for allocate new memory dynamically for new nodes to insert in linked-list.`* you need new memory for new nodes to add in linked list `malloc()` is a system call that ask runtime-system to allot it a memory of given size. — Grijesh Chauhan, Jul 26 '13 at 19:26
@PranabBoruah see every time you call `insert()` you update `root` to either `root->left` or `root->right` so in current ruining `root` don't have memory — Grijesh Chauhan, Jul 26 '13 at 19:30
ok..so as iam recursively calling the insert() function it needs to malloc a node for creating a new node right?..and the first time it ran properly as i had a node that was malloc in main..am i right? — Pranab Boruah, Jul 26 '13 at 19:30
Yes, yes you are checking insert that whether root is null or not so you are correct — Grijesh Chauhan, Jul 26 '13 at 19:32

score 1 · Answer 3 · answered Jul 26 '13 at 16:21

1

malloc gets some memory and returns its address. If you don't store that address in the variable, the variable has NULL (0) in it. When you store to that address (using the pointer), its not a legal memory location and the processor complains with that error.

answered Jul 26 '13 at 16:21

Lee Meador

12,829
2
36
42

But that's not what happens. – Jiminion Jul 26 '13 at 16:25
@Jim Which part do you disagree with ... in words for an obvious newby OP. – Lee Meador Jul 26 '13 at 16:26
The program doesn't seem to crash right away. He makes it through one iteration. – Jiminion Jul 26 '13 at 16:27
@Jim I took "after taking the first value" to mean "the first time it calls insert() after the fscanf() reads a value' but you may be right. Storing to 0 usually fails right away but maybe not if this is some sort of embedded machine it runs on. – Lee Meador Jul 26 '13 at 16:34
1

I'm hoping this is why the question is marginally interesting; he was able to write to location 0x000004 (or whatever) and it didn't die right away. – Jiminion Jul 26 '13 at 16:36

score 0 · Answer 4 · answered Jul 26 '13 at 16:27

You seem to like throwing memory out of the window ! In main:

struct node *root= malloc(sizeof(struct node ));  // reserve some memory
if(root==NULL)                                    // make sure that memory is reserved
{
    printf("mem error");
    return;
}
root=NULL;                                        // loose the address of the memory

This should just be:

struct node *root = NULL;

Jiminion · Answer 5 · 2013-07-26T16:43:31.540

0

When root to NULL, addressing root->key put you in a very problematic part of memory. Your program managed to survive one iteration before crashing.

edited Jul 26 '13 at 16:43

answered Jul 26 '13 at 16:29

Jiminion

5,080
1
31
54

coping data to a node value in c gives segmentation fault but malloc removes it

5 Answers5