Вопрос управления памятью для начинающих

Question

ОК, у меня есть этот код код :

typedef struct faux_crit
{
  char dna[DNALEN+1]; //#define'd to 16
  int x, y;
  int age;
  int p;
  int dir;
} crit;

crit *makeguy(int x, int y)
{
  crit *guy;
  guy = (crit *) malloc(sizeof(crit));
  strcpy(guy->dna, makedna());
  guy->x = x;
  guy->y = y;
  guy->age = guy->p = guy->dir = 0;
  return guy;
}

char *makedna()
{
  char *dna;
  int i;
  dna = (char *) malloc(sizeof(char) * DNALEN+1);
  for(i = 0; i < DNALEN; i++)
    dna[i] = randchar();
  return dna;
}

int main()
{
  int i;
  crit *newguy;
  srand((unsigned) time(0));

  newguy = makeguy(0, 0);
  /*[..]
   just printing things here
   */
  free(newguy);

  return 0;
}

Я бы просто хотел узнать, что я не так сделал с управлением памятью, потому что valgrind сообщает об ошибке памяти. Я предполагаю, что это dna var в makedna, но когда мне его освободить? У меня нет доступа к нему после выхода из функции, и мне нужно вернуть его, поэтому я не могу освободить его до этого.

РЕДАКТИРОВАТЬ: Хорошо, спасибо всем.

Answer 1

Самый простой способ - изменить makeguy () следующим образом:

char* dna = makedna();
strcpy(guy->dna, dna);
free(dna);

Но это не очень хорошее решение, поскольку вы выделяете память в одном месте и освобождаете ее в другом. Лучше сделать malloc и бесплатно в одном месте. Поэтому я рекомендую изменить makedna () на:

void* makedna(char* dna, int dna_len)
{
  int i;
  for(i = 0; i < dna_len; i++)
    dna[i] = randchar();
}

Вы можете вызвать makedna () следующим образом:

char* dna = (char*)malloc(DNALEN+1);
makedna(dna, DNALEN);
dna[DNALEN] = 0;
strcpy(guy->dna, dna);
free(dna);

Теперь makedna () делает только то, что от нее требуется: создать последовательность ДНК. Об управлении памятью должен позаботиться абонент. Кроме того, это решение дает гибкость использования статического массива символов, если это требуется на другом сайте вызова.

Answer 2

Вы должны сделать это:

char *tempdna = makedna();
strcpy(guy->dna, tempdna);
free(tempdna);

Но для работы strcpy ваша функция makedna должна завершать строку нулем. В конце, перед возвратом, есть:

dna[DNALEN] = 0;

Answer 3

Вы должны сохранить указатель makedna() перед передачей его на strcpy, чтобы вы могли освободить его, когда закончите.

char* dna = makedna();
strcpy(guy->dna, dna);
free(dna);

Answer 4

Вы должны изменить makedna (), чтобы получить параметр:

void makedna(char* dna)
{
  int i;
  for(i = 0; i < DNALEN; i++)
  {
    dna[i] = randchar();
  }
}

обратите внимание на дополнительные скобки для ясности

и строка 14 становится:

makedna(guy->dna);

Это позволяет избежать хотя бы одного набора возни с malloc и free.
Edit:
Также это решение позволяет избежать любых проблем с нулевым завершением с помощью strcpy.

Answer 5

fbereton прав - вам не нужен маллок для ДНК.

Однако, если бы вы хранили память malloc в своей структуре, ключевым моментом является то, что вы не можете просто освободить структуру и ожидать освобождения других выделений памяти. Вы хотели бы освободить все выделения внутри структуры перед освобождением самой структуры.

Answer 6

Вы правильно догадались. Проблема связана с функцией makedna. Однако в этой функции нет ничего плохого. Это больше о том, как вы используете его.

Совершенно нормально, если makedna возвращает указатель на выделенную память. Тем не менее, вызывающая функция должна освободить эту память после того, как это будет сделано с ней.

Таким образом, я рекомендую вам изменить:

crit *guy;
guy = (crit *) malloc(sizeof(crit));
strcpy(guy->dna, makedna());

до:

crit *guy;
guy = (crit *) malloc(sizeof(crit));
char * dna = makedna();
strcpy(guy->dna, dna);
// Now that we copied the content of the dna string
// we can free it.
free(dna);

РЕДАКТИРОВАТЬ: Как Крис Джестер-Янг указывает , также была небольшая проблема с вашей функцией makedna. Вам нужно завершить нулем значение, которое вы возвращаете из этой функции, чтобы strcpy мог работать правильно. Добавьте это в функцию makedna, прежде чем вернуть результат:

dna[DNALEN] = 0;

Answer 7

Строка 3: dna [DNALEN + 1] // статически объявленный массив, содержащий элементы [DNALEN + 1] - память уже назначена Строка 25: нет необходимости в malloc, поскольку массив уже существует