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;
}
CodePudding user response:
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.