Program crashes when trying to scan a string

Question

I'm having trouble scanning data from a .dat file containing arbitrary game results (for testing the program)

The results are formatted as follows: (# representing the score as an integer)

team_a # team_b # team_a # team_b # team_a # team_b # . . . team_a # team_b #

Each row is a different game.

What I'm trying to do with my code at the moment is to use the fgets() function to scan each game/row, then use the sscanf_s() function to take the data from each row (seeing as though i know the way it is formatted) and store it into the data structure i've defined.

I'm more than happy to take any advice on changes i should make to the way i'm going about getting the data into the struct if there is an easier, faster and/more reliable (foolproof) way to do it.

Any help is greatly appreciated.

Plase read [Why is `while(!feof(inp2b))` always wrong](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). — Iharob Al Asimi, Oct 07 '15 at 07:11
See my wrong answer there, it's because you don't use a white space after every `,` your code is hard to read because of that. It's hard to see where an identifier ends or where the next one starts, so `game_results[i].first_team_name,&game_results[i].first_team_score` made me think of `game_results[i], first_team_name, &game_results[i].first_team_score` which as you can see would make my answer correct — Iharob Al Asimi, Oct 07 '15 at 07:16
I think the problem might be with `sscanf_s`, Microsoft's safe `sscanf`. It expects a buffer size after each string argument, i.e. after arguments for the `%s`, `%c` and `%[` format. See the description and example [here](https://msdn.microsoft.com/de-de/library/t6z7bya3.aspx). — M Oehm, Oct 07 '15 at 07:16
That's another thing, if you are learning use ANSI C. Specially if you are going to ask here. Many of SO users don't have access to a MSVC compiler like myself, and I can't compile your code to test it. — Iharob Al Asimi, Oct 07 '15 at 07:17
As pointed out by M Oehm, your `sscanf_s` looks suspucious, please, read [Things to Avoid in C/C++ -- feof()](http://www.gidnetwork.com/b-58.html): What has happened is the last line of the file was read, but not the EOF. When the loop returns, the `fgets()` attempts to read one more record and failed. — David Ranieri, Oct 07 '15 at 07:20
`data_file_line` can hold 15 lines and `game_results` can hold 128 results yet you use the same index variable, `i` for both. If the loop goes more than 15 loops you have a buffer overflow in `data_file_line`. — Klas Lindbäck, Oct 07 '15 at 07:44
Turn on compiler warnings such as `gcc -Wall -W -Werror` or `clang -Weverything`. Some of your mistakes in the above code may be detected by the compiler. — chqrlie, Oct 07 '15 at 08:13
Trying to take it all on board, thanks for all the comments (y) — Cody Skoumbourdis, Oct 08 '15 at 07:30

score 2 · Accepted Answer · answered Oct 07 '15 at 07:45

Microsoft's secure sscanf_s has a slightly different way of interpreting format specifiers and arguments: In order to prevent buffer overflows, each string format (%s, %c and %[) must pass the corresponding buffer size after the buffer.

So your scan command should read:

    sscanf_s(data_file_line[i],
        "%s %d %s %d",
        game_results[i].first_team_name,
        sizeof(game_results[i].first_team_name),
        &game_results[i].first_team_score,
        game_results[i].second_team_name,
        sizeof(game_results[i].second_team_name),
        &game_results[i].second_team_score);

There are some other issues with your code:

You should check the return value of sscanf_s so you know that the line has been parsed successfully. The return value is the number of items converted, so in your case it should be 4. Also note that %s scans words and team names like "Man Utd" and "Stoke City" have two words and will not parse correctly.
As others have noted, the feof construct will make you read the file once too many. Forget about foef and use the return values of the reading functions instead. For eample, fgets returns NULL when the end of the file is reached, so you can use that as loop condition: while (fgets(buf, sizeof(buf), f)) ...
You don't check whether i overflows. If you have a long file, i might not be big enough.
If you are parsing and storing the lines right away, there's no need to have an array of lines; just use one line buffer over and over.

Ahhhhh kay, i was wondering about what -https://msdn.microsoft.com/de-de/library/t6z7bya3.aspx- meant by buffer, and how to implement it. I'll fix all that up and see how i go. Also, team names such as Man Utd for example are represented in the data file as 'ManUtd' so that makes life a bit easier. Thanks heaps! — Cody Skoumbourdis, Oct 08 '15 at 07:32

score 0 · Answer 2 · answered Oct 07 '15 at 07:39

I simplified a little and it works fine. Your work is not finished. Please try this,

 #include <usual.h>

 #define MAX_NAME_CHARS 15
 #define MAX_DATA_FILE_LINE_LENGTH 32
 #define MAX_GAME_RESULTS 128

 int main( void )
 {
   FILE *inp2b;

   typedef struct game_results
   {
     char first_team_name[MAX_NAME_CHARS];
     int first_team_score;
     char second_team_name[MAX_NAME_CHARS];
     int second_team_score;
   } game_results_t;

   game_results_t game_results[MAX_GAME_RESULTS];
   char data_file_line[MAX_DATA_FILE_LINE_LENGTH][MAX_DATA_FILE_LINE_LENGTH];
   int errorcode = 0;
   int i = 0;

 //errorcode = fopen_s(&inp2b,"C:\\Users\\Cody\\Documents\\Visual Studio 2012\\DATAFILES FOR PA2\\input2b.dat","r");
   inp2b = fopen( "C:\\testdat\\input2b.dat", "r" );

   if ( inp2b == NULL )
     errorcode = 1;

   if ( errorcode != 0 )
   {
     printf( "Error opening 2nd data file!\n\n" );
     return ( 0 );
   }
   else
   {
     printf( "\n\n\nFile was opened successfully!\n\n" );
   }

   i = 0;

   while ( !feof( inp2b ) )
   {
     fgets( data_file_line[i], MAX_DATA_FILE_LINE_LENGTH, inp2b );
     puts( data_file_line[i] );
     printf( "\n" );
     //   sscanf_s(data_file_line[i],"%s %d %s %d",game_results[i].first_team_name,&game_results[i].first_team_score,game_results[i].second_team_name,&game_results[i].second_team_score);
     sscanf( data_file_line[i], "%s %d %s %d", game_results[i].first_team_name,
        &game_results[i].first_team_score,
        game_results[i].second_team_name,
        &game_results[i].second_team_score );

     printf( "\n\n %s %d %s %d \n\n", game_results[i].first_team_name,
        game_results[i].first_team_score,
        game_results[i].second_team_name,
        game_results[i].second_team_score );
     i++;
   }

   fclose( inp2b );

   return ( 0 );
 }

`while(!feof(inp2b))` is still the wrong logic for parsing a file, since you do no check the return value of `fgets`, whatever you do after that uses indetermined data. — chqrlie, Oct 07 '15 at 08:10
You are right, I said : the program now runs but you still have work to do, I am trying to overcome the first problem first : the crash. Cheers. — BobRun, Oct 07 '15 at 08:23
Using the changes made in this answer and the above answer to get it working. Thanks mate — Cody Skoumbourdis, Oct 08 '15 at 07:33

Program crashes when trying to scan a string

2 Answers2