tags:

views:

98

answers:

5
struct DVDInfo  *ReadStruct( void ) {
    struct DVDInfo  *infoPtr;
    int             num;
    char            line[ kMaxLineLength ];
    char            *result;

    infoPtr = malloc( sizeof( struct DVDInfo ) );

    if ( NULL == infoPtr ) {
        printf( "Out of memory!!!  Goodbye!\n" );
        exit( 0 );
    }

    printf( "Enter DVD Title:  " );
    result = fgets( line, kMaxLineLength, stdin );
    line[ strlen( line ) - 1 ] = '\0';
    infoPtr->title = MallocAndCopy( line );

    printf( "Enter DVD comment:  " );
    result = fgets( line, kMaxLineLength, stdin );
    line[ strlen( line ) - 1 ] = '\0';
    infoPtr->comment = MallocAndCopy( line );

    do {
        printf( "Enter DVD Rating (1-10):  " );
        scanf( "%d", &num );
        Flush();
    }
    while ( ( num < 1 ) || ( num > 10 ) );

    infoPtr->rating = num;

    printf( "\n----------\n" );

    return( infoPtr );
}

I asked a different question about this code in another thread on stackoverflow but didn't want to double up on that one - why is the terminating zero being added to the end of these files read in by fgets? fgets adds the terminating zero anyway, isn't this overkill?

+2  A: 

Generally, you replace the newline character that fgets adds to the string with a NUL character. In all cases, fgets will NUL-terminate.

See: http://www.opengroup.org/onlinepubs/009695399/functions/fgets.html

Michael Goldshteyn
fgets always terminates with a 0;
Peter G.
@Michael, not always. If you pass length as 0 then it won't. Crazy corner case though.
JaredPar
+1  A: 

Your

result = fgets( line, kMaxLineLength, stdin );

is Ok since the size of line is kMaxLineLength.

fgets reads in at most one less than size characters from stream and stores them into the buffer ...

pmg
+1  A: 
Steve M
+1 for checking the return value of `fgets`.
pmg
+1  A: 

fgets writes a nul terminator into the buffer you provide (if you specify the buffer size as larger than 0). Otherwise you could not call strlen() on it, strlen() expects a string, and if it isn't nul terminated it is not a string.

You're asking about

line[ strlen( line ) - 1 ] = '\0';

This strips off the last character in line .If you've read a line, it replaces the last character, presumably a \n with a nul terminator.

Consider that fgets just read a line, e.g. your line buffer now contains the string "Hello\n" (the \n is just the escape sequence here, it's actually just 1 character, not 2)

strlen ("Hello\n") is 6, and 6-1 is 5, so the 5. index is replaced by 0

"Hello\n"
      ^
      |
      Add 0 terminator

Result: "Hello"

Just be careful:

  • you don't want to do line[ strlen(line) - 1 ] = '\0'; on an empty string, in that case you'll end up doing line[-1].
  • You should check if fgets succeds. You don't want to poke around in line if fgets failed, and didn't write anything to your buffer.
  • You might want to check whether a whole line actually got read. IF the line you read is larger than kMaxLineLength ,or e.g. if the last "line" in the file doesn't have a trailing \n , strlen(line) -1 will not be a \n (newline).
nos
Your last bullet point should also account for the last line in a file without a trailing newline.
jamesdlin
A: 

Yes, it's overkill.

One suggestion to make it more robust against code rot... change

result = fgets( line, kMaxLineLength, stdin );

to

result = fgets( line, sizeof(line), stdin );
kotlinski