views:

117

answers:

3

I want to 'construct' (read: malloc and memset) my hashtable in c. To do this, I created a function as follows:

int maketable(struct hash_entry **table, int size){

    table = (struct hash_entry **)malloc(size*sizeof(struct hash_entry *));
    int i = 0;
    for (; i<size; i++) {
        memset(table[i], '\0', sizeof(struct hash_entry *));
    }
    return 0;

}

Given that table will have been declared as

struct hash_entry **table[size]

before entering this code, I won't lose anything when I return from maketable, right?

EDIT: Does passing table into maketable() ensure that as long as I change the data that table points to, the changes will be preserved?

EDIT II: I am trying to allocate an array of pointers to pointers to hash_entries

+2  A: 

You will need to assign the result of malloc to * table - otherwise it won't be visible outside the function.

Also, the typical way of using this is to declare a pointer to the hashtable and pass the address of that pointer into the function.

Anon.
how do I do that?
piggles
`* table = (struct hash_entry *) malloc ...`
Anon.
+4  A: 

Your code is assigning to the local table variable, the caller is not affected. This results in a memory leak.

Outside the function, you have declared table as an array of pointer to pointers of struct hash_entry - I'm guessing you rather just want an array of pointers to struct hash entries.

If you actually declare table as an array, there's no need to malloc that space. You just need a loop to set every element in it to NULL (Don't memset each element to zeroes).

If the goal is to allocate the entire table, this is perhps what you're looking for:

struct hash_entry **table;
...
int maketable(struct hash_entry ***table, int size){

    *table = malloc(size* sizeof **table);
    int i = 0;
    for (; i<size; i++) {
       (*table)[i] = NULL;
    }
    return 0;
}

call it like

maketable(&table,100);

I'd rather make that return the table like so:

struct hash_entry ** maketable(int size){
   return calloc(size, sizeof(struct hash_entry *));
}

if the declaration struct hash_entry **table[size] is really what you want, you need to tell us what your maketable() function actually is supposed to do (e.g. do you want to a dynamically allocated 'array' as one of the elements in that table ?

nos
+1  A: 

No. Your types don't match up.

Are you trying to allocate a table of hash entries (i.e., the type of table[i] is struct hash_entry), a table of pointers to hash_entries (i.e., the type of table[i] is struct hash_entry *), or something else? Based on how your code reads, I'm assuming the first case, but let me know if that's wrong.

Assuming you're dynamically allocating a table of struct hash_entry, your declaration of the table in the caller should be

struct hash_entry *table; // 1 *, no array dimension

the function should be called as

int result = maketable(&table, number_of_elements);

and defined as

int maketable (struct hash_entry **table, size_t size)
{
  int r = 0;

  // sizeof **table == sizeof (struct hash_entry)
  *table = malloc(sizeof **table * size);
  // *ALWAYS* check the result of malloc()
  if (*table)
  {
    size_t i;
    for (i = 0; i < size; i++)
      memset(&(*table)[i], 0, sizeof (*table)[i]);
    r = 1;
  }
  return r;
}

A couple of things to point out. First of all, don't cast the result of malloc(). As of C89, you don't need to, and the cast will supress a diagnostic if you forget to include stdlib.h or otherwise don't have a prototype for malloc() in scope. Secondly, you can use the sizeof operator on objects instead of types. This can help reduce some maintenance headaches (i.e., if you change the type of table in the parameter list, you don't have to change the sizeof calls along with it).

Finally, note that the address of table is being passed to the function; since we're trying to write to a pointer value, we must pass a pointer to that pointer.

If you were attempting to create a table of pointers to struct hash_entry, the code is mostly the same, just an extra level of indirection:

your declaration of the table in the caller should be

struct hash_entry **table; // 2 *, no array dimension

the function should be called as

int result = maketable(&table, number_of_elements);

and defined as

int maketable (struct hash_entry ***table, size_t size)
{
  int r = 0;

  // sizeof **table == sizeof (struct hash_entry *)
  *table = malloc(sizeof **table * size);
  // *ALWAYS* check the result of malloc()
  if (*table)
  {
    size_t i;
    for (i = 0; i < size; i++)
      (*table)[i] = NULL;
    r = 1;
  }
  return r;
}

EDIT There was a bug in the maketable examples; table needs to be dereferenced before applying the subscript, i.e., (*table)[i]. We're applying the subscript to what table points to, not the table pointer itself.

Sorry for any confusion.

John Bode
I am trying to allocate a table of pointers to pointers to hash_entries.
piggles
Then you need to declare the table as `struct hash_entry ***table;`. The type of the parameter in the function needs to be `struct hash_entry ****table`. Everything else should be the same as the second example.
John Bode