2
struct node {
    void *info;
    struct node *nextNode;
}

struct linked {
    struct node* Head;
    struct node* Tail;
}

struct linked* pro[32]

void initialize_list()
     printf("seg 1\n");
     for (int i = 0; i < 32; i++) {
        if(!(pro[i] = (struct linked *) malloc(sizeof(struct linked)))) {
        pro[i]->Head = pro[i]->Tail = NULL;
        }
     }
     printf("seg 2\n");
}

I don't know why but when I call the initialize_list() function in main

int main() {
    initialize_list();
}

I sometimes get the output:

seg 1 
segmentation fault

and sometimes

seg 1 
seg 2

What I mean by sometimes, is that when I run the program for 8 times lets say, it might run with the correct output in 6 runs and in 2 runs it might produce the segmentation fault output. Can you help me try to figure out what is wrong with this initialization? and why is it making some memory leaks I suppose?

Hashim Alhasanii
  • 43
  • 4

2 Answers2

1
if(!(pro[i] = (struct linked *) malloc(sizeof(struct linked)))) {

should be

if((pro[i] = (struct linked *) malloc(sizeof(struct linked)))) {

or better yet (don't cast malloc):

if((pro[i] = malloc(sizeof(struct linked)))) {

otherwise you tell to initialize the members when malloc fails

Notice that you can also use calloc, in such case there is no need to initialize the members to NULL:

 for (int i = 0; i < 32; i++) {
     pro[i] = calloc(1, sizeof(struct linked));
     if (pro[i] == NULL) // And always check the return
     {
         perror("calloc");
         exit(EXIT_FAILURE);
     }
 }
David Ranieri
  • 39,972
  • 7
  • 52
  • 94
  • but will I need to set the head and tail to NULL after using calloc? so after the if statement? – Hashim Alhasanii Nov 25 '20 at 13:41
  • No, there is no need to initialize `head` and `tail` with `NULL` when `calloc` is used, `calloc` fill all bytes with zeroes and a pointer with all bytes set to zero is equivalent to `(void *)0` --> `NULL` in practice. – David Ranieri Nov 25 '20 at 13:53
1

The problem is with your comparison:

if(!(pro[i] = (struct linked *) malloc(sizeof(struct linked)))) {
    pro[i]->Head = pro[i]->Tail = NULL;
}

This will only evaluated to true if malloc return NULL. So if the condition is true, you're attempting to dereference a NULL pointer. This invokes undefined behavior which means your program may or may not crash.

You instead want to assign to pro[i]->Head if malloc does not return NULL. Also, there's no need to cast the return value of malloc.

if((pro[i] = malloc(sizeof(struct linked)))) {
    pro[i]->Head = pro[i]->Tail = NULL;
}
dbush
  • 205,898
  • 23
  • 218
  • 273