Search code examples
cpointersmallocfree

where and how do I correctly free malloc'd pointers?


I am in a C programming course right now, and am making a shell similar to bash. Right now, I am working on implementing piping. For this reason, I needed to strndup the piped command to not modify the original piped command when parsing it. At the end, I tried to free the malloc'd pointer, but then my program does not work correctly. Can someone please inform me of what I am doing wrong? Below is the code for the while loop for my parser:

   while (pipeSeparatedCommand != NULL) {
        char *duplicateCmdForPiping = strndup(pipeSeparatedCommand, strlen(pipeSeparatedCommand)); // duplicates command
        pipeSeparatedCommand = strtok(NULL, "|"); // starts at the NULL Character that was tokenized
        char *token = strtok(duplicateCmdForPiping, " "); // finds the first space in the command, tokenizes the arguments by this space
        int index = 0;  // counts the number of commands and arguments

        while (token != NULL) {
            if ((input != NULL) || (output != NULL)) { // advances to the next argument if input or output is found (does not include them in args)
                token = strtok(NULL, " "); 
                continue;
            }

            commandArgsCount[index]++;
            commandArray[numberOfPipes][index] = token; // sets the argument index equal to the address of token (containing the token)
            token = strtok(NULL, " "); // token will search for the next delimiter from the last delimiter end spot
            index++;    
        }

        commandArray[numberOfPipes + 1][index] = NULL; // prevents the args array from collecting garbage in memory.

        if (pipeSeparatedCommand != NULL) // catches the zero case
            numberOfPipes++; // this begins at zero, increments if more pipes need to be added

        free(duplicateCmdForPiping);
        duplicateCmdForPiping = NULL;
    }

Solution

  • Check the man page for strtok(). It says the function will return "a pointer to the beginning of each subsequent token in the string." I suggest you change the line

        commandArray[numberOfPipes][index] = token;
    

    to

        // make sure you `#include <string.h>`
        strcpy(commandArray[numberOfPipes][index], token);
        // OR
        commandArray[numberOfPipes][index] = strdup(token);
    

    By the way please avoid using camel-case variable naming convention in C/C++. It makes your code extremely hard to read.