C strcpy crashes code

Question

I have the following code, the code crashes whenever the strcpy function is called. When these lines are commented out the code does not crash. What is wrong?

char cities[80][17];
char char_distances[40][2];
int distances[40];
char cities_sorted[20][17];

while (!feof(Text)) {
    fscanf(Text, "%[^\t]\t%[^\t]\t%[^\n]\n",
           cities[i], cities[i + 1], char_distances[i]);
    distances[i] = atoi(char_distances[i]);
    printf("City_start: %s  City_end: %s  Distance: %d \n",
           cities[i], cities[i + 1], distances[i]);
    static char uniqueCities[21][17];
    int uniqueCitiesCount;
    for (int j = 0; j < 21; j++) {
        printf("%s\n", uniqueCities[i]);
        bool start_unique = !areEqual(cities[i], cities[j]);
        bool end_unique = !areEqual(cities[i], cities[j + 1]);
        if (start_unique) {
            strcpy(uniqueCities[uniqueCitiesCount], cities[i]);
            uniqueCitiesCount++;
        }
        if (end_unique) {
            strcpy(uniqueCities[uniqueCitiesCount], cities[i + 1]);
            strcpy(uniqueCities[uniqueCitiesCount], cities[i + 1]);
            uniqueCitiesCount++;
        }
    }
    i++;
}

Thanks

are you sure cities[i + 1] does not go out of range for the array? can you try gdp to see at which comparison it crashes with which index values — koksalb, Feb 17 '18 at 15:17
Several problems here. Where are the pointers at `uniqueCities[i]` initialized? — lurker, Feb 17 '18 at 16:11
`while (!feof(Text)) {` https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong — Ed Heal, Feb 17 '18 at 17:42
Check the return values from `fcanf` - read that manual page — Ed Heal, Feb 17 '18 at 17:43
`atoi` is not safe - consider when the string is not a number — Ed Heal, Feb 17 '18 at 17:45

score 1 · Answer 1 · answered Feb 17 '18 at 17:50

What's wrong?

Well many things:

you posted a code fragment: this is not enough information to get help diagnosing your problem. The code posted cannot be compiled and tested, does not even have definitions for all the symbols in the fragment: how is Text defined? how was it opened? is it guaranteed to be different from NULL?
i is not defined in the fragment, how is it defined? is it initialized?
while (!feof(Text)) is not a good way to test for end of input, you should instead compare the result of fscanf() with the expected number of conversions.
fscanf(Text, "%[^\t]\t%[^\t]\t%[^\n]\n", does not have enough information to avoid buffer overflows. The maximum number of characters to store into the arrays should be specified this way: fscanf(Text, "%16[^\t]\t%16[^\t]\t%1[^\n]\n",. Note however that if the input is not consistent with these limitations, conversions will fail and the rest of the input file will be read out of sync. You shoud read the lines into a line buffer and use sscanf() to parse the lines.
char char_distances[40][2]; defines an array of array that can only contain 1 character and a null terminator. The distances must all be expressed as a single digit in the input file. You should probably define the arrays with a larger size or convert the distance directly into the distances array with a %d conversion specifier.
int uniqueCitiesCount; defines a local variable that is not initialized. Using it in your loop invokes undefined behavior as you probably try to access beyond the end of the 2D array.
printf("%s\n", uniqueCities[i]); will print an empty string as no city has yet been copied to this array.
bool start_unique = !areEqual(cities[i], cities[j]); how are bool and areEqual() defined?
strcpy(uniqueCities[uniqueCitiesCount], cities[i + 1]); is duplicated. The index i + 1 is incorrect.
The logic in the loop is contorted and probably flawed. You should just compare the city name with all entries in the uniqueCities and only add it the this array after the loop if it has not been found.

C strcpy crashes code

1 Answers1