CODEWITHSUNDEEP
cpointersmalloc
Igor K
Igor K

Reputation: 95

Why does this malloc not work in C?

Just trying to make a kind of hash table with each node being a linked list.

Having trouble just initializing the space, what am I doing wrong?

#include <stdlib.h>

typedef struct entry {
 struct entry *next;
 void *theData;
} Entry;


typedef struct HashTable {
 Entry **table;
 int size;
} HashTable;

int main(){
 HashTable *ml;
 ml = initialize();
 return 0;
}

HashTable *initialize(void)
{
 HashTable *p;
 Entry **b;
 int i;


 if ((p = (HashTable *)malloc(sizeof(HashTable *))) == NULL)
  return NULL;
 p->size = 101;

 if ((b = (Entry **)malloc(p->size * sizeof(Entry **))) == NULL)
         return NULL;

 p->table = b;

 for(i = 0; i < p->size; i++) {
  Entry * b =  p->table[i];
  b->theData = NULL;
  b->next = NULL;
     }

 return p;
}

Upvotes: 0

Views: 977

Answers (4)

Eric Brunstad
Eric Brunstad

Reputation: 21

Also, you can statically declare your HashTable, either locally, or in some region high enough in the stack that the stack is non-ascending (in memory) while it is used and pass a pointer to the HashTable to your initialize function to avoid a malloc. malloc is slow.

So in main, you can do:

HashTable table; InitializeHashTable(&table);

// use table (no need to free) // just do not return table

Upvotes: 0

CashCow
CashCow

Reputation: 31435

Firstly your sizeofs are wrong. T * = malloc( num * sizeof(T)) is correct. You can also use calloc.

You are reusing b for different purposes so it is quite confusing. Not generally good using a single character variable.

p->table which was b is allocated but not initialised, i.e. it doesn't point to anything useful, then you are trying to dereference it.

You need to fill it will Entry* pointers first, and they must be pointing to valid Entry structs if you are going to dereference those.

Your process probably dies on the line b>theData = NULL

Upvotes: 0

Mitch Wheat
Mitch Wheat

Reputation: 300489

 if ((p = malloc(sizeof(HashTable))) == NULL) 
  return NULL; 
 p->size = 101;  

 if ((b = malloc(p->size * sizeof(Entry *))) == NULL) 
         return NULL;

I believe removing the malloc() result casts is best practice.

Plus, as @Naveen was first to point out you also need to allocate memory for each Entry.

Upvotes: 1

Naveen
Naveen

Reputation: 73433

You need to change sizeof(HashTable*) to sizeof(HashTable) and similarly sizeof(Entry **) to sizeof(Entry *) . And the second thing is for every Entry you need to allocate memory using malloc again inside the loop.

Upvotes: 9

Related Questions