Trying to learn proper memory handling in C -- malloc, realloc and free

Question

So I have two (hopefully quick) questions. I think I've got the hang of using malloc to save space for data, but realloc is causing trouble. In the code below, I have an array of 8 character pointers which -- should it fill up -- I'm trying to extend to have another 8 character pointers (and then another 8, and so on). Realloc does this the first time (ie, it will extend the array once), but after that I get the following error:

*** glibc detected *** ./a.out: realloc(): invalid next size:

As far as I can tell, nothing is changing. Why would realloc work on an array of 8, but not on an array of 16?

And my second question is about memory leaks. I'm still unsure about what I need to free in a program. I've been advised by others that inputcpy needs to be freed. Is that all here? Also, at what point in the program do I want to free it?

#define DEBUG 1

#include <string.h>
#include <stdlib.h>
#include <stdio.h>

char ** getArgs( char *input,  char **args, int ct);
char ** args;

int main(int argc, char* argv[]) {
  char input[]="echo arg1 arg2 arg3 arg4 arg5 arg6 arg7 arg8 arg9 arg10 arg11 arg12 arg13";
  char inputcpy[strlen(input)];
  strcpy(inputcpy, input);
  char * prog=strtok(input, " ");

  /*Saving space for an array of 8 strings*/
  args=( char **) calloc(8, sizeof( char *));  

  getArgs(inputcpy, args, 1);

  if(DEBUG) {
    printf("arg address after: %p\n", args);
  }

  int q;
  int pid=fork();
  if (pid==0) {
    execvp(prog, args); 
    return 0;
  }
  else {
    int status=0;
    wait(&status);
  }
}

char ** getArgs( char *input,  char **args, int ct) {
  int adj=(ct-1)*8;//if we recurse, this ensures correct indexes are used
  char *inputcpy=malloc(strlen(input));
  strcpy(inputcpy, input);

  /*Initialize indexes/Prepare for copying*/
  int i; 
  if(ct==1) {
    i=1;
    args[0]=" "; //quick hack to ensure all args are used by exec()
  }
  else
    i=0;

  /**Actually do the copying now**/
  char *temp=strtok(NULL, " ");
  args[adj+i++]=temp;
  while (temp != NULL && i<8) {
    temp=strtok(NULL, " ");
    args[adj+i++]=temp;
  }   

  /*If there are more args than we have room for*/
  if(i>=8){
    /*Increase the array to store 8 more strings*/
    args= (char **) realloc(args, sizeof(args)+8*sizeof(char *) );
    getArgs(inputcpy, args, (++ct) );
  }   

  return NULL;
}

Answer 1

I will answer your last question, which it was: Also, at what point in the program do I want to free it?

Well, it's recommendable that you free it right before the return stament of the function that called malloc. If you do that, you won't have to worry about any memory leak latter.

However, the calling function, in your case main, can also call free() when your getArgs() function is finished and returns control to main. This is because it is okay to use a different pointer variable with free(); you just have to make sure that the new pointer stores the same address.

Answer 2

 char *inputcpy=malloc(strlen(input));
 strcpy(inputcpy, input);

you are not allocating enough room for your string, should be: malloc(strlen(input) + 1);

Same here:

 char inputcpy[strlen(input)];
 strcpy(inputcpy, input);

A string is a sequence of characters terminated and including a null character and the length of a string is the number of characters preceding the null characters.

Also you are using realloc incorrectly:

args= (char **) realloc(args, sizeof(args)+8*sizeof(char *) );

There is a potential memory leak in this use.

Regarding free, what you should do is simple: every malloc should have a corresponding free.