Search code examples
cstringtext-filesstdio

After reading from text file, results are displayed incorrectly


I've written a program that reads four variables (three strings and one character) every line from a text file. But when I display the variables, an unexpected character pops up at the end of each line. (I've ensured that the lengths of the variables are large enough).

Why is this? (Overflowing buffers, again?) And how do I fix this?

Text file contents:

M0001 Cool Name F 123-456789
M0002 Name Cool M 987-654321

Code:

    #include <stdio.h>
    #include <stdlib.h>

    int main() {
        FILE *text;

        char id[6], name[101], gender, contact[13];

        text = fopen("test.txt", "r");
        while (fscanf(text, "%s %[^\n]s %c %s\n", id, name, &gender, contact) != EOF)
            printf("%s %s %c %s\n", id, name, gender, contact);
        fclose(text);

        return 0;

}

The output I expect:

M0001 Cool Name F 123-456789
M0002 Name Cool M 987-654321

What I get instead:

M0001 Cool Name F 123-456789 1⁄4
M0002 Name Cool M 987-654321 1⁄4

Solution

  • in the call to fscanf(), the format string: "%s %[^\n]s %c %s\n" is not correct.

    1. the '[^\n]' will read to the end of the line (which will overflow the input buffer: `name'. Then the next char is NOT an 's' because the next character is the newline.
    2. should compare the returned value to 4, not EOF
    3. the input/format specifiers '%[...]' and '%s' have no problem overflowing the input buffer, so should ALWAYS have a MAX_CHARACTERS modifier that is one less than the length of the input buffer (those format specifiers always append a NUL byte to the input

    The following proposed code:

    1. cleanly compiles
    2. documents why each header file is included
    3. performs the desired functionality
    4. splits the 'name' into 'firstname' and 'lastname' for easier handling and to match the format of the input data
    5. properly checks the returned value from fscanf()
    6. properly checks for any error from fopen() and if an error is returned, properly outputs the error message and the text indicating why the system thinks the function failed to stderr
    7. uses an appropriate format string for the calls to fscanf() and printf()
    8. replaces 'magic' numbers with meaningful names via a enum statement

    And now the proposed code:

    #include <stdio.h>   // fopen(), fclose(), fscanf(), perror(), printf()
    #include <stdlib.h>  // exit(), EXIT_FAILURE
    
    
    enum{
        MAX_ID_LEN = 6,
        MAX_NAME_LEN = 20,
        MAX_CONTACT_LEN = 13
    };
    
    
    int main( void )
    {
        char id[ MAX_ID_LEN ];
        char firstname[ MAX_NAME_LEN ];
        char lastname[ MAX_NAME_LEN ];
        char gender;
        char contact[ MAX_CONTACT_LEN ];
    
        FILE *text = fopen("test.txt", "r");
        if( !text )
        {
            perror( "fopen to read 'test.txt' failed" );
            exit( EXIT_FAILURE );
        }
    
        // implied else, fopen successful
    
        while (5 == fscanf(text, "%5s %19s %19s %c %12s",
            id, firstname, lastname, &gender, contact) )
        {
            printf("%s %s %s %c %s\n",
                id, firstname, lastname, gender, contact);
        }
    
        fclose(text);
        return 0;
    }