strtok not going through all tokens

Question

I'm trying to implement a shell as part of a school assignment, and I'm stuck on the file input/output redirection part.

More specifically, I've come up with a function which allows me to detect whether or not the command entered in specifies a '>' or '<' or even a '|'.

Ideally, if I enter ls -a > ls.tx', then the tokens ls -a and ls.txt should be returned.

My code doesn't do this, it only returns ls -a then stops.

My code is below:

/*commandLine is a char* taken in from the user, and is a null-terminated string */
int counter = 0;
parsedLine = strtok(commandLine, ">");
while (parsedLine != NULL)
{
    if (counter == 0)
    {
        strncpy(parsedCpy, parsedLine, strlen(parsedLine));
        parseCommand(parsedCpy, commands);
        counter++;
    }
    else
    {
        redirect->re_stdout = parsedLine;
    }
    parsedLine = strtok(NULL, ">");
}

I've tried it in another test file just to see if there was something wrong, but this test file (code below) returns the expected result (that is, ls -a and ls.txt)

    char myString[] = "ls -a > ls.txt";
char* parsed;

parsed = strtok(myString, ">");
while (parsed != NULL)
{
    printf("%s\n", parsed);
    parsed = strtok(NULL, ">");
}

Is there something that I'm just not understanding? I don't really see where I'm going wrong, since the code itself is nearly the same in both cases.

Answer 1

Note that strncpy won't zero terminate a string unless the zero termination is part of the source being copied. See man strncpy. It says:

Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated.

That could be horsing something else up depending upon what parseCommand does.

In this case, you should just do a strcpy. A strncpy doesn't really do anything for you if you're giving it the length of the source string, unless you're intentionally trying to avoid copying the null terminator. So you should use, strcpy(parsedCpy, parsedLine);.

Answer 2

I cannot see how parsedLine is declared, but it needs to be handled explicitly and carefully. i.e. make sure the pointer to that value is not changed except by strtok(), and make sure that it remains null terminated. One thing I do when using strtok() for multiple calls, is to use an intermediate value to collect results, helping to keep the target buffer pure and unchanged except by strtok()

A small code snippet to illustrate:

char a[] = {"ls -a > ls.tx"};
char *buff;
char keep[80];

buff = strtok(a, ">");
    strcpy(keep, buff);
buff = strtok(NULL, ">");
    strcat(keep, buff);    

This usage of strtok() is clean, i.e. it does not allow buff to be affected except by another call to strtok()

By comparison, this section of your code is a little scary because I do not know the output of the strncpy() which depends so heavily on the third argument, and can corrupt (place unexpected results into) parseCommand :

if (counter == 0)
{
    strncpy(parsedCpy, parsedLine, strlen(parsedLine));
    parseCommand(parsedCpy, commands);
    counter++;
}
else
{
    redirect->re_stdout = parsedLine;
}
parsedLine = strtok(NULL, ">");

Along the lines of keeping the target buffer pure, (even though it does not appear to be an issue here), strtok() is not thread safe. If a function using strtok() is used in a multi threaded process, the target buffer is subject to any number of calls, resulting in unexpected, and perhaps even undefined behavior. In this case using strtok_r() is a better option