Issues with strtok in C

Question

I have the following function in my code to extract names:

void student_init(struct student *s, char *info) {
    char *name;
    char *name2;
    char a[] = { *info };
    name = strtok(a, "/");
    strcpy(s->first_name, name);
    name2 = strtok(NULL, "/");
    strcpy(s->last_name, name2);
}

When I run this, I see:

Please enter a student information or enter "Q" to quit.
Daisy/Lee
A student information is read.: Daisy/Lee
Please enter a row number where the student wants to sit.
1
Please enter a column number where the student wants to sit.
2
The seat at row 1 and column 2 is assigned to the student: D . �
?.?. ?.?. ?.?.
?.?. ?.?. D.�.
?.?. ?.?. ?.?.

I am trying to use the strtok function in a c program to split a string with "/" to separate a fist and last name and store them in the first_name and last_name variables of a student strcuture. I can get the first name stored in the respective variable but as you can see from the image in the link above I am getting a ? symbol in the output where the first initial of the last name should be.

Welcome to SO! Please read [tour] then [ask] then [mcve] then edit your question. — Dave S, Jan 30 '18 at 01:11
Please don't post pictures of code and output, we cannot copy&paste code from a picture and reproduce it ourselves. — Pablo, Jan 30 '18 at 01:16
`char a[] = { *info };` doesn't do what you think it does. Since `info` is a `char *`, `*info` is a single character. — David Schwartz, Jan 30 '18 at 01:55

paxdiablo · Accepted Answer · 2018-01-30T02:24:14.157

char a[] = { *info };

This is your problem. What this creates is a one-byte character array containing the first character of info and nothing else.

Since strtok requires a string, it's probably going to run off the end of that one-byte array and use whatever happens to be there in memory. That's why you're seeing the first character okay and not much else (though, technically, as undefined behaviour, literally anything is allowed to happen).

Rather than constructing a one-byte array, you should probably just use the string as it was passed in:

name = strtok(info, "/");

The only reason you would make a local copy is if the string you're tokenising was not allowed to be changed (such as if it were a string literal, or if you wanted to preserve it for later use). Since your sample run shows that you're reading into this string, it cannot be a string literal.

And, if you want to preserve it for later, that's probably a cost best incurred by the caller rather than the function (it's wasteful for the function to always make a duplicate when the information as to whether it's needed or not is known only to the caller).

In order to make a copy for tokenising, it's as simple as:

char originalString[] = "pax/diablo";

scratchString = strdup(originalString);
if (scratchString != NULL) {
    student_init (struct student *s, scratchString);
    free (scratchString);
} else {
    handleOutOfMemoryIssue();
}

useSafely (originalString);

If your implementation doesn't have strdup (it's POSIX rather than ISO), see here.

In my opinion, a "cleaner" implementation would be along the lines of:

void student_init (struct student *s, char *info) {
    // Default both to empty strings.

    *(s->first_name) = '\0';
    *(s->last_name) = '\0';

    // Try for first name, exit if none, otherwise save.

    char *field = strtok (info, "/");
    if (field == NULL) return;
    strcpy (s->first_name, field);

    // Try for second name, exit if none, otherwise save.

    if ((field = strtok (NULL, "/")) == NULL) return;
    strcpy (s->last_name, field);
}

The problem with that is that `strtok` modifies its input. The caller likely isn't expecting that, so it is much safer to copy the input to the stack first. — Turn, Jan 30 '18 at 02:10

Issues with strtok in C

1 Answers1