You have a number of issues... Your program will be exhibiting undefined behaviour currently, so until you address the issues you cannot hope to predict what's going on. Let's begin.
The following string is one character too short. You forgot to include a character for the string terminator ('\0'
). This will lead to a buffer overrun during tokenising, which might be partly responsible for the behaviour you are seeing.
char copy[strlen(commandline)]; // You need to add 1
strcpy(copy, commandline);
The next part is your return value, but it's a temporary (local array). You are not allowed to return this. You should allocate it instead.
// Don't do this:
char* commands[length];
for (int x=0; x<length; x++)
commands[x] = "\0"; // And this is not the right way to zero a pointer
// Do this instead (calloc will zero your elements):
char ** commands = calloc( length, sizeof(char*) );
It's possible for the tokenising loop to overrun your buffer because you never check for length
, so you should add in a test:
while( temp != NULL && i < length )
And because of the above, you can't just blindly set commands[i]
to NULL after the loop. Either test i < length
or just don't set it (you zeroed the array beforehand anyway).
Now let's deal with the return value. Currently you have this:
return *commands;
That returns a pointer to the first token in your temporary string (copy
). Firstly, it looks like you actually intended to return an array of tokens, not just the first token. Secondly, you can't return a temporary string. So, I think you meant this:
return commands;
Now, to deal with those strings... There's an easy way, and a clever way. The easy way has already been suggested: you call strdup
on each token before shoving them in memory. The annoying part of this is that when you clean up that memory, you have to go through the array and free each individual token.
Instead, let's do it all in one hit, by allocating the array AND the string storage in one call:
char **commands = malloc( length * sizeof(char*) + strlen(commandline) + 1 );
char *copy = (char*)(commands + length);
strcpy( copy, commandline );
The only thing I didn't do above is zero the array. You can do this after the tokenising loop, by just zeroing the remaining values:
while( i < length ) commands[i++] = NULL;
Now, when you return commands
, you return an array of tokens which also contains its own token storage. To free the array and all strings it contains, you just do this:
free( commands );
Putting it all together:
void* setCommands(int length)
{
// Create array and string storage in one memory block.
char **commands = malloc( length * sizeof(char*) + strlen(commandline) + 1 );
if( commands == NULL ) return NULL;
char *copy = (char*)(commands + length);
strcpy( copy, commandline );
// Tokenise commands
int i = 0;
char *temp = strtok(copy, " \t");
while( temp != NULL && i < length )
{
commands[i++] = temp;
temp = strtok(NULL, " \t");
}
// Zero any unused tokens
while( i < length ) commands[i++] = NULL;
return commands;
}