Where is my memory leak not being freed in this shell simulation?-CodePudding

I can't find where my memory leak isn't being freed in a shell simulation C program using the valgrind tool. I also ran my program using the gdb debugger. writableUsersArgs contained "/usr/bin/vim" as the first element and NULL as the second element. Using the debugger, it showed the array writableUsersArgs was freed twice.

Function containing the memory leak

int start(char* command){
        // pid uses status's value behind the scenes
        int status;
        int maxArgLength = 0;
        int argLength = 0;
        int index;

        // Split the user's command into seperate strings wherever the command has spaces
        char** usersArgs = commandDelimeter(command);
        int numberArgs = countArgs(command);
        // Calculate maxArgLength to allocate correct amount of memory
        for(index = 0; index < numberArgs; index  ) {
                argLength = countLongestArg(usersArgs[index]);
                if(argLength > maxArgLength) {
                        maxArgLength = argLength;
                }
        }
        // Allocate memory for the array that gets passed to exec() function
        char* writableUsersArgs[numberArgs 1];
        for(index = 0; index < numberArgs   1; index  ) {
                writableUsersArgs[index] = (char*)malloc(maxArgLength*sizeof(char));
        }
        // Copy the strings from the read only array to a writable array of strings
        for(index = 0; index < numberArgs; index  ) {
                strcpy(writableUsersArgs[index], usersArgs[index]);
        }
        // Last entry of array must be NULL for exec() function to work properly
        writableUsersArgs[index] = NULL;

        // Create a child process
        pid_t pid = fork();
        if(pid == -1) {
                printf("Error forking");
                return -1;
        } else if(pid == 0) {
                // Jumps into new child process
                execv(writableUsersArgs[0], writableUsersArgs);
                return 2;
        } else {
                // Waits for child process to terminate before proceeding
                wait(&status);
                for(index = 0; index < numberArgs   1; index  ) {
                        free(usersArgs[index]);
                        free(writableUsersArgs[index]);
                }
                free(usersArgs);
                return 1;
        }
}

Valgrind memory check error messages

==670== HEAP SUMMARY:
==670==     in use at exit: 26 bytes in 2 blocks
==670==   total heap usage: 73 allocs, 71 frees, 8,432 bytes allocated
==670==
==670== 13 bytes in 1 blocks are definitely lost in loss record 1 of 2
==670==    at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==670==    by 0x10ABC3: createArgsArray (mysh.c:579)
==670==    by 0x10A946: commandDelimeter (mysh.c:508)
==670==    by 0x109DB0: start (mysh.c:269)
==670==    by 0x109820: executeCommandWithArgs (mysh.c:151)
==670==    by 0x109641: executeCommand (mysh.c:115)
==670==    by 0x109542: insideShell (mysh.c:83)
==670==    by 0x1094C3: main (mysh.c:63)
==670==
==670== 13 bytes in 1 blocks are definitely lost in loss record 2 of 2
==670==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==670==    by 0x109EDB: start (mysh.c:281)
==670==    by 0x109820: executeCommandWithArgs (mysh.c:151)
==670==    by 0x109641: executeCommand (mysh.c:115)
==670==    by 0x109542: insideShell (mysh.c:83)
==670==    by 0x1094C3: main (mysh.c:63)
==670==
==670== LEAK SUMMARY:
==670==    definitely lost: 26 bytes in 2 blocks
==670==    indirectly lost: 0 bytes in 0 blocks
==670==      possibly lost: 0 bytes in 0 blocks
==670==    still reachable: 0 bytes in 0 blocks
==670==         suppressed: 0 bytes in 0 blocks

CodePudding user response：

In start you allocate memory with malloc and assign it to a pointer, then you overwrite the content of the pointer with NULL. You have now lost track of the malloced memory.

So, in this loop:

for(index = 0; index < numberArgs   1; index  ) {
    writableUsersArgs[index] = (char*)malloc(maxArgLength*sizeof(char));
}

every member of writableUsersArgs is assigned a pointer to a unique memory block reserved by malloc, including the array member writableUsersArgs[numberArgs].

A few lines later, you write

writableUsersArgs[index] = NULL;

Her index has the same value as numberArgs. You have now lost all references to the malloced block that writableUsersArgs[index] previously pointed at.

You probably wanted to run the loop while index < numberArgs.

This only accounts from the lost block in start. I cannot say anything about the lost block in commandDelimeter or createArgsArray unless you show that code. But I suspect it is the same problem.

Just a note at the end: I see no need to copy the strings returned by creareArgsArray before calling fork/exec