views:

570

answers:

4

Updated question is here

http://stackoverflow.com/questions/828108/please-help-me-to-solve-this-memory-allocation-problem

I'm working on making a HashTable in C. This is what I've done. I think I'm going on a right path but when I'm trying to

main.c

HashTablePtr hash;
hash = createHashTable(10);
insert(hash, "hello");
insert(hash, "world");

HashTable.c

    HashTablePtr createHashTable(unsigned int capacity){
    HashTablePtr hash;
    hash = (HashTablePtr) malloc(sizeof(HashTablePtr));
    hash->size = 0;
    hash->capacity = capacity;
    ListPtr mylist = (ListPtr)calloc(capacity, sizeof(ListPtr)); /* WHY IT DOESN'T ALLOCATE MEMORY FOR mylist HERE?? */
    mylist->head = NULL;
    mylist->size = 0;
    mylist->tail = NULL;    
    hash->list = mylist;  
    return hash;

ListPtr is a LinkedList ptr

List.h

typedef struct list List;
typedef struct list * ListPtr;

struct list {
    int size;
    NodePtr head;
    NodePtr tail;
};
...
...

HashTable.h

    typedef struct hashtable * HashTablePtr;
    typedef struct hashtable HashTable;
    struct hashtable {
        unsigned int capacity;
        unsigned int size;
        ListPtr *list;
        unsigned int (*makeHash)(unsigned int, void *);
    };
...
...

When I run my debugger, I see no memory being allocated to myList. In above example, my attempt is to make it an array of 10 lists.

Please help me to solve this.

I'm not that expert in C, if that helps.

A: 

You are allocating a contigous block of ListPtr's, but you actually want to allocate space for the all the structures rather than just pointers (ListPtr) to those structures:

calloc(capacity, sizeof(List));

I agree with gman's comment about not hiding pointers. When coding in C, I never typdef a List * as a ListPtr. It makes the code harder to understand.

Mitch Wheat
+2  A: 
calloc(capacity, sizeof(ListPtr)

should be

calloc(capacity, sizeof(List)
Steve Lazaridis
+2  A: 

I think there are a whole host of problems here. You haven't included the error you getting so, I will list a couple:

  • hash = (HashTablePtr) malloc(sizeof(HashTablePtr*)); - You are allocating the size of a HashTable **, which is four bytes, you need to allocate the size of the underlying object.
  • ListPtr mylist = (ListPtr* )calloc(capacity, sizeof(ListPtr)); - Again, your are allocating the size of the pointer rather than the underlying list object.
  • HashTablePtr createHashTable(unsigned int capacity)){ - You are probably getting a compile error here with the extra parens and the inconsistent number of parameters.
Cannonade
sorry but I wanted to make in look cleaner so screwed it up. I'm not getting any compile error.
+1  A: 

Personally I am not a huge fan of using typedefs, especially when you are a beginner. I think that may be partially what is confusing you. You are better avoiding things like:

typedef struct hashtable * HashTablePtr;

Using to many typedefs will make your code harder to read since you will need to constantly look up what they are referring too.

The main problem is that you were allocating the memory for the size of a hashtable/list pointer and not for the size of their respected structures. I think the code below shows this well. You will also want to check if you allocations worked. If malloc, calloc, realloc. etc. fail they return NULL. If this happens and you do not check for this case you will get a segfault error and your program will crash.

Also follow the c99 standard, and put all of your variable declarations at the start of the function.

c99 std

malloc manpage

struct hashtable *
createHashTable(unsigned int capacity){
    struct hashtable *hash;
    struct list *mylist;

    /* You want to allocate size of the hash structure not the size of a pointer. */
    hash = malloc(sizeof(struct hashtable)); 
    // always make sure if the allocation worked.
    if(hash == NULL){
        fprintf(stderr, "Could not allocate hashtable\n");
        return NULL;
    }

    hash->size = 0;
    hash->capacity = capacity;

    /* Unless you need the memory to be zero'd I would just use malloc here
     * mylist = calloc(capacity, sizeof(struct list)); */
    mylist = malloc(capacity * sizeof(struct list));
    if(mylist == NULL){
        fprintf(stderr, "Could not allocate list\n");
        free(hash); /* free our memory and handle the error*/
        return NULL;
    }

    mylist->head = NULL;
    mylist->size = 0;
    mylist->tail = NULL;    
    hash->list = mylist;

    return hash;
}

Also remember to free you list before you free your hashtable:

free(myhash->list);
free(myhash);
Sweeney