Nodes using Structs

Question

typedef struct{

      int key;
      int priority;
}array_node;


array_node *newNode(int key, int priority) {

     array_node *g;
     g = (array_node *)calloc(1,sizeof(array_node));

     if (NULL==g) {
        fprintf(stderr, "Out of mem!\n");
        return (NULL);
     }

     g->key=key;
     g->priority=priority;
     return g;
}

int main(){

    array_node *newNode;
    newNode->key = 5;
    newNode->priority = 1000;

    printf("%d\n",newNode->key);
}

Hi everyone! I need an insight on why I get segmentation fault whenever i compile the program. It seems like everything is fine but i don't know where the error is coming from.

I'm just implementing a node structs.

I guess `malloc(sizeof(array_node *))` should be changed into `malloc(sizeof(array_node))`. — timrau, Jul 19 '12 at 16:38
FYI I was still editing the post so there wasn't any questions yet. — ms. sakura, Jul 19 '12 at 16:40
@ms.sakura: next time please fully write the question before posting. — orlp, Jul 19 '12 at 16:40

score 3 · Answer 1 · edited May 23 '17 at 10:34

3

Firstly:

g = (array_node *)malloc(sizeof(array_node *));

It should be

g = malloc(sizeof(array_node));

In first one you allocate memory for array_node pointer, but you need array_node object. When you later tried to do

g->key=key;
g->priority=priority;

You were referencing to somewhere in memory, but not to object component. Of course you unnecessary casted malloc return value to (array_node*), because in C you don't have to cast from void*. Here you can read about it.

Another error even more serious, because that is where your seg fault come from:

array_node *newNode;
newNode->key = 5;
newNode->priority = 1000;

Should be as another responder pointed out:

array_node* node = newNode(5, 1000);

In your version you just create a pointer to array_node object (just an address to memory), so you can't refer to key or priority because they don't exists.

Now i see that you changed malloc to

array_node *g;
g = (array_node *)calloc(1,sizeof(array_node));

Calloc you should use when you want to allocate memory for an array. In your case you should use malloc, which you can find above.

edited May 23 '17 at 10:34

Community

1
1

answered Jul 19 '12 at 16:40

Blood

4,126
3
27
37

2

going to also comment that if this is C, you shouldn't be casting that malloc. – ardent Jul 19 '12 at 16:40
but if i don't cast it i get this error: my_heaps.c:78: error: invalid conversion from ‘void*’ to ‘array_node*’ – ms. sakura Jul 19 '12 at 16:43
That means you write in C++, not C. Unless you use C++ compiler. Either way correct your malloc and everything should work fine. – Blood Jul 19 '12 at 16:44
1

@catchmeifyoutry, that inconsistency has been edited into this answer :), which was really just masking the problem that the malloc was creating a block of memory the size of an array_node pointer. – ardent Jul 19 '12 at 16:54
@ms.sakura, if you're coding in C++ you might as well use "new" rather than "calloc", otherwise you should be using malloc to allocate space for structs like these. – ardent Jul 19 '12 at 16:59
He write in C, but i think that he compile in C++ :D – Blood Jul 19 '12 at 17:00

score 3 · Answer 2 · answered Jul 19 '12 at 16:40

3

Try this

// use these headers
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct{
  int key;
  int priority;
}array_node;

array_node *newNode(int key, int priority) {

  // sizeof(array_node), not sizeof(array_node) == sizeof(void*) == 4 or 8 typically
  array_node *g = malloc(sizeof(array_node));

  if (NULL==g) {
     fprintf(stderr, "Out of mem!\n");
     return (NULL);
  }

  g->key=key;
  g->priority=priority;
  return g;
}

int main(){
   /// call the function, not just type its name
   array_node* n = newNode(5, 1000);

   printf("Key = %d, Priority = %d\n", n->key, n->priority);

   return 0;
}

Watch for closing brackets, it is not Python, it is C. The printf() will not bother thinking for you. It will just print the address of n.

answered Jul 19 '12 at 16:40

Viktor Latypov

14,289
3
40
55

I followed everything you posted here but i get this error: my_heaps.c: In function ‘array_node* newNode(int, int)’: my_heaps.c:78: error: invalid conversion from ‘void*’ to ‘array_node*’ – ms. sakura Jul 19 '12 at 17:02
okay I was able to fix the problem by adding this array_node *g = (array_node *)malloc(sizeof(array_node)); Thanks so mch – ms. sakura Jul 19 '12 at 17:05
Then just leave the cast to (array_node*). The compiler you're using treats your file as C++ source, not plain C. – Viktor Latypov Jul 19 '12 at 17:14

score 2 · Answer 3 · answered Jul 19 '12 at 16:40

2

Look in main():

array_node *newNode;

This pointer has an undefined value. The correct is:

array_node *node = newNode(5, 1000);

answered Jul 19 '12 at 16:40

Probably because originally there was another mistake that was also causing problems; only making this change wouldn't have fixed the code. – Dennis Meng Jul 19 '12 at 16:50
1

Not entirely a good reason for downvoting this answer rather than upvoting the other answer though. – ardent Jul 19 '12 at 16:53
i get this error: my_heaps.c: In function ‘int main()’: my_heaps.c:96: error: ‘newNode’ cannot be used as a function – ms. sakura Jul 19 '12 at 16:55
+1 Because, based on the posted code, the `newNode()` function is irrelevant, since it is never called. The fact is a pointer is declared, but left uninitialized, and no memory is allocated, but then the (either garbage or NULL) pointer is dereferenced. – twalberg Jul 19 '12 at 16:57
@ms.sakura Either you left `array_node *newNode;` unchanged, or you renamed the `newNode()` function. – catchmeifyoutry Jul 19 '12 at 16:58
@DennisMeng the other mistake was indeed a bug, but didn't cause any problems exactly because of what this answer describes. Whoever downvoted this did it without understanding what was going on. – catchmeifyoutry Jul 19 '12 at 17:01
I'm not saying I agree with whoever downvoted this; I was simply giving what I thought was a possible reason. – Dennis Meng Jul 19 '12 at 17:12

Nodes using Structs

3 Answers3