Why is the first char* null in this char **?

Question

I am working on a few functions that will work together to initialize a list of char *'s as a command list. I am storing them in a char **, which I want to alter whenever it's necessary to insert another command. When I run the insert command with a single char *command, It says that commandList[0] = "ls" inside of the insert function, but in the initializeCommands function it says that testCommands[0] = NULL. Can anyone explain why this is happening?

void insertCommand(char *command, char **commandList){
    if(commandList[0] == NULL){ // no commands
        commandList = (char **)malloc(sizeof(char *) * 2);
        commandList[0] = (char *)malloc(strlen(command) * sizeof(char));
        commandList[0] = command;
        printf("Commandlist[0]: %s\n", commandList[0]);
        commandList[1] = NULL;
    } else { // one or more commands
        int i = 0;
        while(commandList[i] != NULL){
            i++;
        }
        commandList = (char **)realloc(sizeof(char *) * (i + 1));
        commandList[i - 1] = (char *)malloc(sizeof(char) * strlen(command));
        commandList[i] = NULL;
    }
    return;
}

The printf statement above shows that commandList[0] != NULL

char **initializeCommands(){
    char **testCommands = createCommandList();
    insertCommand("ls", testCommands);
    printf("testCommand[0]: %s\n", testCommands[0]);
    return testCommands;
}

The printf statement above shows that testCommands[0] == NULL

create command list

char **createCommandList(){
    char **commandList = (char **)malloc(sizeof(char *));
    commandList[0] = NULL;
    return commandList;
}

In C you cannot copy a string with an assignment. You must use `strcpy` to copy the string in the freshly allocated memory. Instead of `commandList[0] = command;` use `strcpy(commandList[0] , command);`. Moreover you have to allocate space for the terminating null, so `commandList[0] = (char *)malloc(strlen(command) + 1);`. You don't need the `sizeof(char)` which is always the base size. — Frankie_C, Jul 16 '22 at 19:24
`sizeof(char)` is always 1 and redundant in `sizeof(char) * strlen(command)`. — 273K, Jul 16 '22 at 19:26

score 1 · Accepted Answer · answered Jul 16 '22 at 19:38

C uses pass by value.

commandList is a local variable in insertCommand, completely independent of testCommands in initializeCommands (except commandList is initialized from testCommands once, at the beginning of the execution of insertCommand). Assigning a new value to commandList has no effect on testCommands or anything else outside of insertCommand. Whatever value testCommands has before the call to insertCommand, it will have after the call.

The interface of insertCommand is completely broken. A function that allocates a block of memory to be consumed outside should return a pointer to it. (Or, alternatively, assign it to something visible outside of the function, but don't do this unless you cannot return a value for some reason).

The implementation of insertCommand is broken too because there is a memory leak.

if(commandList[0] == NULL){ 
    commandList = ....  // <--- here

This throws the previous value of commandList out of the window (and there is a previous value, otherwise commandList[0] would be illegal).

In fact, either the "then" part of that "if" or the whole createCommandList is completely redundant. If you are using createCommandList, just the else part of that if will work just fine. Alternatively, you can get rid of createCommandList and just pass NULL as an empty list. Don't forget to change if(commandList[0] == NULL) to if(commandList == NULL) if you do.

Yet another problem is inconsistency between the "then" part and the "else" part.

commandList[0] = command;

This copies a pointer.

commandList[i - 1] = (char *)malloc(sizeof(char) * strlen(command));

This allocates a whole new buffer, but you forgot to copy command to it, and you also forgot about the null terminator.

Why is the first char* null in this char **?

1 Answers1