0

My main question is, is my scheme just plain bad practice? Can it be done? Should it be done? I'm writing a little dinky key-value pair "dictionary" structure just to familiarize my self with C. One of the functions I've written is intended to return an array of strings of all the values associated with a provided key. The function definition:

char** get_values(const struct dict* d, const char* key)
{
  // if key does not exist
  if(contains_key(d, key) == 0)
    {
      return NULL;
    }
  // count the number of times the key occurs in the dict
  int count = count_keys(d, key);
  // make a return value
  char** key_values = alloc_list(count);// definition included below

  int curr_count = 0;
  // fill return array
  for(int pos = 0; pos < DSIZE; pos++)
    {// if current key in dict matches requested key...
      if(strcmp(d->keys[pos], key) == 0)
        {
          // add current value to return array
          strcpy(key_values[curr_count], d->values[pos]);
          curr_count++;
        }
    }
  return key_values;
}

This function allocates the memory for the string array:

static char** alloc_list(int count)
{
  // Allocate list of strings
  char** slist = (char**)malloc(sizeof(char*) * count);
  // if allocation was great success...
  if(slist)
    {
      // ... allocate memory for each string
      for(int pos = 0; pos < DSIZE; pos++)
        {
          slist[pos] = (char*)malloc(DSIZE * sizeof *slist[pos]);
        }
    }
  return slist;
}

Then in main():

 add(&dictionary, "a", "a");
 add(&dictionary, "a", "aa");
 add(&dictionary, "a", "aaa");

 char** a_val = get_values(&dictionary, "a");  // TODO: how do I free this!

 assert(strcmp(a_val[0], "a") == 0);
 assert(strcmp(a_val[1], "aa") == 0);
 assert(strcmp(a_val[2], "aaa") == 0);  // all these assertions pass

 free(*a_val);  // with * omitted, execution aborts, with * included, no warnings 
                      // from gcc, yes I am stabbing in the dark here.
 a_val = NULL;

I don't believe the last two lines are doing what I hope they are doing, when I print the values of a_val[0-2] in gdb, they are still there. I realize I could fix this problem by allocating a string array in main(), and then change get_values() to accept the array and then let get_values() do its business, and then free() the allocated array of strings when I am done with it. But before I go ahead and do that, I was just wanted to see if and how one goes about deallocating memory from a calling function. I've read a little bit about it, and all that was said was "it is the programmers responsibility to deallocate memory in the calling function", but the book did not say how for a situation such as this.

Thanks in advance for any help!

Budge
  • 91
  • 7

1 Answers1

1

In order to properly deallocate a_val you will need first a for-loop to free/deallocate the char arrays allocated previously and then free the double pointer (i.e., a_val). Otherwise you will create a memory leak since the memory pointed by elements/pointers of a_val will be unreferenced/orphaned:

char** a_val = get_values(&dictionary, "a");
...
for(int pos = 0; pos < DSIZE; pos++) {
  free(a_val[pos]);
}
free(a_val);

Stating free(*a_val); is equivalent as stating free(a_val[0]). Thus only the first string of a_val is going to be deallocated.

101010
  • 41,839
  • 11
  • 94
  • 168