Why does this malloc not work in C?

Question

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;
}

As a general point; don't cast the return from malloc() in C. — Williham Totland
– Williham Totland, Commented Oct 26, 2010 at 11:22
Specifically, don't write p = (HashTable *)malloc(sizeof(HashTable *)), but rather p = malloc(sizeof(*p)). The cast is only useful in order to ensure that p has the "right type", i.e. the same type as you used for the size. In which case the cast should be to a type with one more asterisk than the type used in sizeof. Since you've got it wrong here anyway, the cast is doing you no good at all, and can obscure an error message that results from forgetting to include stdlib.h. — Steve Jessop
– Steve Jessop, Commented Oct 26, 2010 at 11:39

Naveen · Accepted Answer · 2010-10-26 11:21:45Z

9

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.

answered Oct 26, 2010 at 11:21

Naveen

73.8k47 gold badges181 silver badges237 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Igor K Over a year ago

Thanks! Hmm, making this change to p for sizeof(HashTable) is giving an error allocating memory to p. Using VS 2010. Will try it again but that seems to be what its saying.

Steve Jessop Over a year ago

I would prefer sizeof(*p) to sizeof(HashTable). Then the line reads, "allocate the right thing for p", instead of "allocate this thing, which I think is the right thing for p but who knows, I might have made a mistake" :-)

Mitch Wheat · Accepted Answer · 2010-10-26 11:22:25Z

1

 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.

answered Oct 26, 2010 at 11:22

Mitch Wheat

302k44 gold badges482 silver badges552 bronze badges

1 Comment

Igor K Over a year ago

How is it I would do this, tried this but still get a memory related error?

CashCow · Accepted Answer · 2010-10-26 11:27:13Z

0

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

answered Oct 26, 2010 at 11:27

CashCow

31.6k5 gold badges65 silver badges97 bronze badges

Comments

Eric Brunstad · Accepted Answer · 2010-10-26 11:57:25Z

0

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

answered Oct 26, 2010 at 11:57

Eric Brunstad

212 bronze badges

1 Comment

Igor K Over a year ago

I will later have to resize the table. The idea was to create a new one, then I can just use the pointer to the new table. Not sure if I can do this if I code it this way? Thanks for the input though, I'm not sure to be honest.

Collectives™ on Stack Overflow

Why does this malloc not work in C?

4 Answers 4

2 Comments

1 Comment

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

1 Comment

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related