1

The question is how to correctly allocate/free the memory in this example:

void test(char*** array, int* count) {

  *array = malloc(sizeof(char*) * MAX_ARRAY);
  while (...) {
    (*array)[i] = (char*)malloc(strlen(fooString));
  }
}

call of the function:

char** array;
int count;
test(&array, &count);
// now free the memory - i think i have to?
for(i = 0; i < count; i++) {
  free(array[i]); // <-- crash here
}
free(array);

It looks like that array[0] has a different address inside the test-function than outside. How can this be? Looks like i misunderstood sth, because the address of array is the same outside and inside the function.

Edit: The Problem is that i am not able to free the allocated memory (see "crash here" in code). Why? And how will it work?

alk
  • 69,737
  • 10
  • 105
  • 255
marty bourque
  • 734
  • 4
  • 7
  • 20
  • 2
    not applicable to your problem, but don't cast the return value of malloc() – KevinDTimm Oct 30 '13 at 12:55
  • array[0] in test is not the same as array[0] outside of test. In test, array holds the address of the variable called array in the function calling test. Also [do not cast the return value of malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Eregrith Oct 30 '13 at 12:56
  • i recognized that these are two different array[0] (still don't understand why), but how am i able to free the allocated memory? – marty bourque Oct 30 '13 at 12:58
  • Another note: `strlen` does not include the '\0' character. – domen Oct 30 '13 at 13:40
  • What is this `while (...)`? – alk Oct 30 '13 at 15:11
  • your test function doesnt set count, so your free loop will run off the end – pm100 Oct 30 '13 at 16:31
  • @marty You're allocating an array of MAX_ARRAY char*, but when you free the emelemts you iterate over `count` elements. if count > MAX_ARRAY, you get in trouble. If you have more code than what's shown here, e.g. a strcpy((*array)[i], fooString), you are off by one, and thrashing the memory. – nos Oct 30 '13 at 16:32

2 Answers2

2

Instead of

void test(char*** array, int* count) {

  *array = malloc(sizeof(char*) * MAX_ARRAY);
  while (...) {
    (*array)[i] = (char*)malloc(strlen(fooString));
  }
}

do

void test(char*** array, int count) {

  *array = malloc(sizeof(char*) * count); // number of pointers
  for (int i = 0; i < count; ++i) 
  {
    (*array)[i] = malloc(strlen(fooString)); 
  }
}

although i am not sure about what fooString is since you don't show the decl/def. Normally you would allocate one byte extra for the \0

(*array)[i] = malloc(strlen(fooString) + 1)

this seems to work

#include <stdio.h>
#include <inttypes.h>
#include <malloc.h>
#include <string.h>

char fooString[256];

void test(char*** array, int count) 
{
  int i = 0;
  *array = malloc(sizeof(char*) * count);
  for (i = 0; i < count; ++i) 
  {
    (*array)[i] = malloc(strlen(fooString)+1);
  }
}

int main()
{
  char** array = NULL;
  int count = 100;
  int i = 0;
  test(&array, count);


  for(i = 0; i < count;++i) 
  {
    free(array[i]); 
  }
  free(array);    
  return 0;
}
AndersK
  • 35,813
  • 6
  • 60
  • 86
0

For your particular problem:

You allocate (*array)[i] which is a char* to strlen(fooString) which is usually equivalent to sizeof(char) * strlen(fooString) : this is error prone. You should use sizeof(*((*array)[i])) in this case to be sure not to miss the correct type.

To free it, loop from i = 0 to i < MAX_ARRAY and call free(array[i]) What you put in place of ... in your code is very important.

In general, when allocating memory, be sure to respect these general ideas:

  • If a functions allocates memory it frees it itself except when it is needed outside afterwards.
  • If a function allocates memory needed outside afterwards, it does just this.

This allows for better code architecture and easier freeing of the memory.

For example:

First point:

void foo()
{
  char *a;

  a = malloc(sizeof(*a) * 5);
  a[0] = 'a';
  a[1] = 'b';
  a[2] = 'c';
  a[3] = 'd';
  a[4] = 0; //or '\0' if you prefer
  do_something_cool(a);
  free(a);
}

The function foo allocates memory, processes it, and frees it.

Second point:

char *halfstrdup(char *str)
{
  int len;
  int i;
  char *newstr;

  len = strlen(str);
  newstr = malloc(sizeof(*newstr) * len / 2)
  for (i = 0; i < len; i++)
  {
    if ((i % 2) == 0)
      newstr[i / 2] = str[i];
  }
  return (newstr);
}

void foo2()
{
  char *half;

  half = halfstrdup("Hello, world !");
  do_something_cooler(half);
  free(half);
}

The function halfstrdup just allocates and sets the memory you need and returns it, the function foo2 allocates memory through the use of halfstrdup, then uses it and frees it.

Do not forget to free before losing track of your pointers, for example after returning from foo or foo2, you won't be able to free the allocated memory.

Eregrith
  • 4,263
  • 18
  • 39
  • thanks for the long answer, but this does not handle my problem. – marty bourque Oct 30 '13 at 13:20
  • How does it not handle your problem? Have you looked at least the first part of my answer? – Eregrith Oct 30 '13 at 13:21
  • My problem is a call-by-reference allocation and i am not able to free it again. your first example: everything in same function. second example: return value. No call by reference. – marty bourque Oct 30 '13 at 13:24
  • There is no such thing as "references" in C. What you pass to your function is the address of your variable `array`, you store there the address of `malloc`'d memory, where you store other malloc'd memory addresses. You have to first free the pointers contained in the first allocated piece of memory, which is your array "cells". – Eregrith Oct 30 '13 at 13:55