C initial memory management issue

Question

Ok, I have this code snippet :

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;
}

I just want to know that I made a mistake in memory management because valgrind reports a memory error. I suppose this is dna var in makeda, but when should I free it? I do not have access to it after exiting the function, and I need to return it, so I can not release it before that.

EDIT: Alright, thanks everyone.

Answer 1

The simplest workaround is to modify makeguy () as follows:

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() , , , : dna. . , char, .

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 .
Edit:
strcpy.

Answer 5

. 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 6

fbereton - malloc .

, malloc'd , , . , .

Answer 7

Line 3: dna [DNALEN + 1] // statically declared array containing the elements [DNALEN + 1] - memory is already assigned Line 25: there is no need for malloc as the array exists