string parsing in C

Question

I'm trying to pass a string to chdir(). But I always seem to have some trailing stuff makes the chdir() fail.

#define IN_LEN  128

int main(int argc, char** argv) {

    int counter;
    char command[IN_LEN];
    char** tokens = (char**) malloc(sizeof(char)*IN_LEN);
    size_t path_len; char path[IN_LEN];

      ...

    fgets(command, IN_LEN, stdin) 
    counter = 0;
    tmp = strtok(command, delim);
    while(tmp != NULL) {
     *(tokens+counter) = tmp;
     tmp = strtok(NULL, delim);
     counter++;
    }

    if(strncmp(*tokens, cd_command, strlen(cd_command)) == 0) {
     path_len = strlen(*(tokens+1));
     strncpy(path, *(tokens+1), path_len-1); 
    // this is where I try to remove the trailing junk... 
    // but it doesn't work on a second system
     if(chdir(path) < 0) {
      error_string = strerror(errno);
      fprintf(stderr, "path: %s\n%s\n", path, error_string);
}

// just to check if the chdir worked
char buffer[1000];
    printf("%s\n", getcwd(buffer, 1000));

    }

    return 0;
}

There must be a better way to do this. Can any help out? I'vr tried to use scanf but when the program calls scanf, it just hangs.

Thanks

Answer 1

It looks like you've forgotten to append a null '\0' to path string after calling strncpy(). Without the null terminator chdir() doesn't know where the string ends and it just keeps looking until it finds one. This would make it appear like there are extra characters at the end of your path.

Answer 2

You have (at least) 2 problems in your example.

The first one (which is causing the immediately obvious problems) is the use of strncpy() which doesn't necessarily place a '\0' terminator at the end of the buffer it copies into. In your case there's no need to use strncpy() (which I consider dangerous for exactly the reason you ran into). Your tokens will be '\0' terminated by strtok(), and they are guaranteed to be smaller than the path buffer (since the tokens come from a buffer that's the same size as the path buffer). Just use strcpy(), or if you want the code to be resiliant of someone coming along later and mucking with the buffer sizes use something like the non-standard strlcpy().

As a rule of thumb don't use strncpy().

Another problem with your code is that the tokens allocation isn't right.

char** tokens = (char**) malloc(sizeof(char)*IN_LEN);

will allocate an area as large as your input string buffer, but you're storing pointers to strings in that allocation, not chars. You'll have fewer tokens than characters (by definition), but each token pointer is probably 4 times larger than a character (depending on the platform's pointer size). If your string has enough tokens, you'll overrun this buffer.

For example, assume IN_LEN is 14 and the input string is "a b c d e f g". If you use spaces as the delimiter, there will be 7 tokens, which will require a pointer array with 28 bytes. Quite a few more than the 14 allocated by the malloc() call.

A simple change to:

    char** tokens = (char**) malloc((sizeof(char*) * IN_LEN) / 2);

should allocate enough space (is there an off-by-one error in there? Maybe a +1 is needed).

A third problem is that you potentially access *tokens and *(tokens+1) even if zero or only one token was added to the array. You'll need to add some checks of the counter variable before dereferencing those pointers.