Help with strtok() in C

Started by
1 comment, last by Serapth 12 years, 6 months ago
I keep getting access violations. Can anyone see what I'm doing wrong? I'm trying to split up a first and last name separated by a space character.

[source]
char buffer[128], firstName[64], lastName[64];
char token = 0;

scanf("%s", buffer);
fflush(stdin);

token = strtok(buffer, " ");

if (token) {
strcpy(firstName, token);

token = strtok(0, " ");

if (token)
strcpy(lastName, token);
else
strcpy(lastName, "\0");
} else {
strcpy(firstName, "\0";
}
[/source]
Advertisement
There are a few things.

First off, that code doesn't even compile.

Line 2 should be a char*.

Line 19 is missing a closing parenthesis.

That gets it to compile.


Next, line 4 doesn't do what you think it does.

It is extremely difficult to make scanf() a safe function to use. It will stop after the first whitespace (space, newline, tab, etc.) The scanf family can leave stuff in the input stream. And finally, it is very easy to have a buffer overrun. If someone just pushed a single letter 128 times it could crash your program (or worse).

That means if you enter two words like "Bob Felton", only "Bob" would be stored in your string.

You need to use something like if( fgets( buffer, 128, stdin) != NULL) { ... }

I assume that is why you have the fflush() on line 5, since you have likely left stdin in an unexpected state in line 4.

Lines 10 and 15 could cause buffer overruns if your strings are longer than expected.

Finally, since this is a code snippit from a larger program, know that strtok() modifies your string as it goes along. If your buffer was originally "Bob Felton", it will now be the string "Bob" since that was the first token.
Not hardened by any means....



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

int main(int argc, char* argv[])
{
char byteBuffer[100];

char *firstName, *lastName;

if(fgets(byteBuffer,100, stdin) == NULL)
return -42;


firstName = strtok(byteBuffer," ");
lastName = strtok(NULL," ");

printf("FirstName: %s LastName: %s",firstName, lastName);
return 0;
}




As frob said, strtok is destructive, so the value in byteBuffer isn't what you think it is at this point.

EDIT: Keep in mind, strtok only returns a pointer to a position in already allocated memory, in our case byteBuffer. If you don't implicitly copy the value returned by strtok to newly malloc'd memory, it will become undefined behavior once byteBuffer goes out of scope.

Also, you would probably be better served using the non-destructive strchr to accomplish what you are doing?

Also, is there a really good reason you are using straight C in the first place? String manipulation in C blows, big time. Even C++ is a vast improvement.

EDIT2: If you are wondering "hey, if strtok modifies my data, how is it I can continue to point at it and get valid results???". Good question grasshopper! What strtok does is replace the deliminators specified in strtok will null terminators, so if your string was "hi there mike" and you choose " " as your deliminator, your string gets modified to "hi\0there\0mike". If this modification is ok, go for it, but if you don't want the source string modified, use strchr.

strtok is also not thread safe, I believe.

This topic is closed to new replies.

Advertisement