Tokenizing a string and return it as an array

Question

I'm attempting to tokenize a passed string, store the tokens in an array and return it. I'm running this on ubuntu. I'm stumped when it comes to this language apparently.

Sample input: coinflip 3

My code thought process reads as follows:

take: string
if string = null: return null
else:
while temp != null
   token[i++] = temp
   temp = get next token
return

Here's my current solution. The delimiter is whitespaces. C hasn't been my strong suit for a while.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

//Clears the screen and prompts the user
void msg()
{
    static int init = 1;
    if(init)
    {
        printf("\e[1;1H\e[2J");
        init = 0;
    }
    printf("%s", "uab_sh > ");

}

//Reads in line
char *readIn(void)
{
    char param[101];
    fgets(param, 101, stdin);
    return param;
}

//parse string - still working out the kinks :)
char **parseString(char *cmd)
{
    char delim[] = " ";
    char* temp = strtok(cmd, delim);
    if (temp == " ")
    {
        return NULL;
    }
    else
    {
        int i = 0;
        char** tokens = malloc(3 * sizeof(char*));
        while (temp != NULL)
        {
            tokens[i++] = temp;
            temp = strtok(NULL, " ");
        }
        for (i = 0; i < 3; i++)
        {
            printf("%s\n", tokens[i]);
        }
        return tokens;
    }
}

//Command
int command(char ** cmd)
{
    int pid;
    if (cmd[0] != NULL)
    {
        pid = fork();
        if (pid == 0)
        {
            exit(0);
        }
        else if (pid < 0)
        {
            perror("Something went wrong...");
        }
    }
    else
        return 1;
}


int main()
{
    char *line;
    char **cmd;
    int stat = 0;
    while (1)
    {
        msg();
        line = readLine();
        cmd = parseString(line);
        stat = command(cmd);
        if (stat == 1)
        {
            break;
        }
    }
    return 0;
}

Current errors:

main.c: In function ‘readIn’:
main.c:24:9: warning: function returns address of local variable [-Wreturn-local-addr]
  return param;
         ^~~~~
main.c: In function ‘parseString’:
main.c:32:11: warning: comparison with string literal results in unspecified behavior [-Waddress]
  if (temp == " ")
           ^~
main.c: In function ‘command’:
main.c:59:9: warning: implicit declaration of function ‘fork’ [-Wimplicit-function-declaration]
   pid = fork();
         ^~~~
main.c: In function ‘main’:
main.c:82:10: warning: implicit declaration of function ‘readLine’; did you mean ‘readIn’? [-Wimplicit-function-declaration]
   line = readLine();
          ^~~~~~~~
          readIn
main.c:82:8: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
   line = readLine();
        ^
main.c: In function ‘command’:
main.c:71:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

Answer 1

Here's a little there-and-back-again recursive solution. This idiom uses the call stack as temporary space in order to only allocate the array of tokens once. Since it requires very little bookkeeping, the code is radically simplified. Not everyone will find that appealing.

#include <stdlib.h>
#include <string.h>
static char** helper(char* token, int argc) {
  char** retval = token ? parseString(strtok(NULL, " "), argc + 1)
                        : malloc((argc + 1) * sizeof *retval);
  if (retval) retval[argc] = token;
  return retval;
}

char** parseString(char* cmd) {
  return helper(strtok(cmd, " "), 0);
}

Of course, a recursive implementation means that a line with really a lot of tokens could overflow the stack. In practice, I don't worry about this because the stack frame for helper is quite small and there are no VLAs, allocas or even uninitialized locals. So stack overflow will (1) require really a lot of tokens, and (2) reliably hit the guard page, if there is one, terminating the process. If you're using an OS without stack guards, you could put a check on recursion depth in the function, since the second argument tracks the depth.

Answer 2

The compiler already reported that this function

//Read-in string
char *readIn(void)
{
    char param[101];
    fgets(param, 101, stdin);
    return param;
}

has undefined behavior because it returns pointer to a local array param that will not be alive after exiting the function.

In this function

char *parseString(char* cmd)
{
    char* temp = strtok(cmd, " ");
    if (cmd == NULL)
    {
        return temp;
    }
    else
    {
        int i = 0;
        char *tokens[3];
        while (temp != NULL)
        {
            tokens[i++] = temp;
            temp = strtok(NULL, " ");
        }
        for (i = 0; i < 3; i++)
        {
            printf("%s\n", tokens[i]);
        }
        return tokens;
    }
}

there is the same problem (if not take into account the wrong implementation) and moreover the type of the returned expression

        return tokens;

does not correspond to the return type of the function because the expression in the return statement has the type char ** while the return type of the function is char *.

I am sure that the most difficult for you is to write the function that splits a string into tokens.

It can look the following way as it is shown in the demonstrative program below. The function allocates dynamically memory for an array of pointers to tokens. If an allocation fails the function returns NULL. Otherwise the function returns pointer to first element of a dynamically allocated array of pointers. The last element of the array contains NULL. This element can be used to determine the number of actual pointers to tokens in the array.

Here you are.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char ** parseString( char *cmd )
{
    char **tokens = malloc( sizeof( char * ) );
    *tokens = NULL;
    size_t n = 1;

    const char *delim = " \t";

    char *p = strtok( cmd, delim );

    int success = p != NULL;

    while ( success )
    {
        char **tmp = realloc( tokens, ( n + 1 ) * sizeof( char * ) );

        if ( tmp == NULL )
        {
            free( tokens );
            tokens = NULL;

            success = 0;
        }
        else
        {
            tokens = tmp;

            tokens[n - 1] = p;
            tokens[n] = NULL;
            ++n;

            p = strtok( NULL, delim );

            success = p != NULL;
        }
    }

    return tokens;
}

int main(void) 
{
    char cmd[] = "Many various and unique commands";

    char **tokens = parseString( cmd );

    if ( tokens != NULL )
    {
        for ( char **p = tokens; *p != NULL; ++p )
        {
            puts( *p );
        }
    }

    free( tokens );

    return 0;
}

The program output is

Many
various
and
unique
commands

Answer 3

Well, excluding the fact that this code is not capable of handling no more than 3 tokens, it has an another basic problem: It will return an illegal pointer to memory. temp and tokens are variables which are within the stack frame of the parseString() function. So when it's execution finishes, those variables will be gone. The ideal solution here is to allocate tokens in the heap.

Here is my solution:

char** parseString(char* cmd)
{
    char delimiters[] = " ";
    char* temp = strtok(cmd, delimiters);
    //If temp is NULL then the string contains no tokens
    if (temp == NULL)
    {
        return NULL;
    }
    else
    {
        int i = 0;
        char** tokens = malloc(3*sizeof(char*));
        while (temp != NULL)
        {
            tokens[i++] = temp;
            temp = strtok(NULL, " ");
        }
        for (i = 0; i < 3; i++)
        {
            printf("%s\n", tokens[i]);
        }
        return tokens;
    }
}