CODEWITHSUNDEEP
carrayscharprintf
AC-5
AC-5

Reputation: 293

Char array value will print as empty unless value is printed when assigned - C

I am working with two functions in my c program and am having trouble getting them to cooperate. My first function iterates through a csv file and uses strtok() to separate tokens based on the ',' delimiter. Each token is saved into a character array of size 3 and if a particular token does not match the goal token, the next line of input is read, tokenized, and the character array is overwritten with the new input tokens.

Here is the global variable used:

char * stateCityZip[3];

Here is the code for the first function:

int zipToCity(char * zip)
{
char line[1024];
char * tok = malloc(20 * sizeof(char));
FILE * file = fopen("cityzip.csv", "r");
while (fgets(line, sizeof(line), file))
{
    //State
    tok = strtok(line, ",");
    stateCityZip[0] = tok;
    //City
    tok = strtok(NULL, ",");
    stateCityZip[1] = tok;
    //Zip
    tok = strtok(NULL, ",");
    stateCityZip[2] = tok;

    if (strcmp(stateCityZip[2], newZip) == 0) {
        //printf("Found %s, %s\n", stateCityZip[0], stateCityZip[1]);
        strlen(stateCityZip[1]));
        return 1;
    }
}
return 0;
}

My second function is simply trying to print the values of stateCityZip. When I print these values, however, they appear as blank. The only way to fix this I'v found is to uncomment the 

//printf("Found %s, %s\n", stateCityZip[0], stateCityZip[1]);

line.

Here is the code for my second function:

int main() {
    printf("City: [%s]", stateCityZip[1]);
    printf("State: [%s]", stateCityZip[0]);

    return 0;
}

Output:

City: []
State: []

Upvotes: 0

Views: 1024

Answers (1)

Daniel Pryden
Daniel Pryden

Reputation: 60997

strtok returns pointers to substrings of its input. So all your tok values are pointers to parts of line, which has automatic storage duration, and so the pointers in stateCityZip all become invalid once zipToCity returns.

(Actually, if there is more than one line in the file, the tok pointers all become invalid as soon as you advance to the next line, as they now point into arbitrary substrings of the new line in the buffer.)

Instead, you should use strdup to allocate a copy of the token as a new string, and save the pointer returned from strdup into stateCityZip.

An important note: the strings allocated by strdup need to freed by free. If stateCityZip is global then you can get away with not freeing them (the last memory will be freed when your process exits). But if zipToCity ever gets called again, it will overwrite the pointers in stateCityZip and leak the corresponding strings. So it would probably be safest to first free() any strings in stateCityZip (if they're NULL, then that's fine, as free(NULL) is a no-op) before assigning new values.

On the subject of memory allocation: your example code has a malloc call for tok which is completely superfluous, and which is guaranteed to leak, since you overwrite the pointer returned from malloc without ever freeing it.

And, on the subject of good coding practice: fopen can fail. You need to check that the returned FILE * is not NULL. (If it is NULL, errno will tell you why.) And you also need to fclose the FILE *, which you don't appear to be doing either.

More generally: none of this code appears to be doing any error checking at all. Every time you call any standard library function (or any function at all, for that matter!) you need to think about how that function could fail to do what it's supposed to do, how you would be able to tell (usually failure behavior is well documented, so make sure to read the docs), and what you want your program to do if it fails. What should happen if the file doesn't exist or can't be read? If it contains a line that isn't comma-delimited, or doesn't contain the expected number of fields? What should happen if you run out of memory halfway through processing the file, or if strdup isn't able to copy the string because you're out of heap memory? If you don't think about these problems, or forget to deal with them, you might get away with it in the "happy case" where everything works, but sooner or later it will come back and bite you -- often at the worst possible time.

Upvotes: 1

Related Questions