Reuse char * pointer needs to free and malloc again?

Question

I would like to implement a main function such as in order to execute system commands. The following code is currently used :

int main(int argc, char *argv[])
{
   size_t cmd_length;
   char *cmd_buffer = NULL; 
   char *file = NULL;
   char *ip = NULL;
   int size;

   if(argc == 3)
   {
       size = strlen(argv[1]);
       file = (char*)malloc((size + 1)*sizeof(char));
       strcpy(file, argv[1]);       
       size = strlen(argv[2]);
       ip = (char*)malloc((size + 1)*sizeof(char));     
       strcpy(ip, argv[2]);     
   }

   cmd_length = snprintf(NULL, 0, "tftp -g -r %s %s", file, ip);
   cmd_buffer = malloc(cmd_length + 1);
   if (cmd_buffer == NULL) 
   {
       return -1; 
   }
   snprintf(cmd_buffer, cmd_length + 1, "tftp -g -r %s %s", file, ip);
   if(system(cmd_buffer) == 0) 
   {
      then ...
   }
   {
      return -1;
   }

   free(cmd_buffer);
   cmd_buffer = NULL;

   cmd_length = snprintf(NULL, 0, "tftp -g -r %s %s", DFT_FILE, DFT_IP);
   cmd_buffer = malloc(cmd_length + 1);
   if (cmd_buffer == NULL) 
   {
       return -1; 
   }
   snprintf(cmd_buffer, cmd_length + 1, "tftp -g -r %s %s", DFT_FILE, DFT_IP);
   if(system(cmd_buffer) == 0) 
   {
      then ...
   }
   {
      return -1;
   }

   free(cmd_buffer);
   free(file);
   free(ip);
   cmd_buffer = NULL;
   file = NULL;
   ip = NULL;
   return 0;
}

Because I need to enter other commands, I am currently using the same cmd_buffer by using free() before reallocating memory. Is it the right way to do ? Some other commands might be required in the future.

Answer 1

Your program can be be greatly simplified if you use a common function to execute the system call. It doesn't even need to use malloc at all. Here's a partial implementation [Please pardon the gratuitous style cleanup]:

#include <stdarg.h>

int
execute_command(const char *fmt,...)
{
    char cmd_buffer[5000];
    int cmd_length;
    va_list ap;

    // NOTE: much simpler to used a fixed size buffer that is larger than
    // needed

    va_start(ap,fmt);
    cmd_length = vsnprintf(cmd_buffer,sizeof(cmd_buffer),fmt,ap);
    va_end(ap);

    if (system(cmd_buffer) != 0)
        return -1;

    return 0;
}

int
main(int argc, char *argv[])
{
    char *file = NULL;
    char *ip = NULL;

    // NOTE: I had to guess the intent if fewer arguments are passed (e.g. just
    // skip as I did here, print message and abort?)
    if (argc == 3) {
        // NOTE: no need to malloc these, but if you did, see strdup(3)
        file = argv[1];
        ip = argv[2];

        execute_command("tftp -g -r %s %s", file, ip);
    }

    execute_command("tftp -g -r %s %s", DFT_FILE, DFT_IP);

    return 0;
}