CODEWITHSUNDEEP
cstdio
Ori Popowski
Ori Popowski

Reputation: 10662

Unexpected behavior of 'fread()', 'fwrite()' and 'fseek()'

I wrote a simple C program, which takes a .txt file and replaces all spaces with hyphens. However, the program enters an infinite loop and the result is endless array of hyphens.

This is the input file:

a b c d e f

This is the file after the process crashes:

a----------------------------------------------------------------------------
----------------------------------------... (continues thousands of times)...

I guess the reason in unexpected behavior of fread(), fwrite() and fseek(), or my misunderstanding of these functions. This is my code:

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

#define MAXBUF 1024

int main(void) {

    char buf[MAXBUF];
    FILE *fp;
    char c;
    char hyph = '-';

    printf("Enter file name:\n");
    fgets(buf, MAXBUF, stdin);
    sscanf(buf, "%s\n", buf);   /* trick to replace '\n' with '\0' */

    if ((fp = fopen(buf, "r+")) == NULL) {
        perror("Error");
        return EXIT_FAILURE;
    }

    fread(&c, 1, 1, fp);

    while (c != EOF) {
        if (c == ' ') {
            fseek(fp, -1, SEEK_CUR); /* rewind file position indicator to the position of the ' ' */
            fwrite(&hyph, 1, 1, fp); /* write '-' instead */
        }
        fread(&c, 1, 1, fp); /* read next character */
    }

    fclose(fp);

    return EXIT_SUCCESS;
}

What is the problem here?

Upvotes: 3

Views: 2114

Answers (3)

Romina Kat
Romina Kat

Reputation: 11

The reason:

For files open for update (those which include a "+" sign), on which both input and output operations are allowed, the stream shall be flushed (fflush) or repositioned (fseek, fsetpos, rewind) before a reading operation that follows a writing operation. The stream shall be repositioned (fseek, fsetpos, rewind) before a writing operation that follows a reading operation (whenever that operation did not reach the end-of-file).

The solution:

You should add "fflush(fp);" after the fwrite line.

Upvotes: 1

JimR
JimR

Reputation: 16113

You have a few problems...

Check what types the standard C library functions return and what that return value means. The std C library defines EOF as the integer -1. Since the full character set is 256 characters and the type char can hold from 0 to 255 (256 diff values) it was necessary to make EOF an integer.

With all that bluster aside... You're also checking for EOF incorrectly.

The problems, spelled out:

You should check the return value from fread

if( fread(&c, 1, 1, fp) != 1 )
{
    // Handle the error
}

// `EOF` is the integer -1.  It will not fit in a char.  So, your while loop becomes endless unless you get a -1 in the data stream

// The "correct" way to do what you want to do is using the stdlib function feof(fp)
while( !feof( fp ) )
{
    if (c == ' ')
    {
        // You should check the value returned by fseek for errors
        fseek(fp, -1, SEEK_CUR); /* rewind file position indicator to the position of the ' ' */
        // You should check the value returned by fwrite for errors
        fwrite(&hyph, 1, 1, fp); /* write '-' instead */
    }

    if( fread(&c, 1, 1, fp) != 1 )
    {
        // Handle the error
    }
}

All of that said... It is very inefficient to read a character at a time on modern systems. Adapt your code to read a buffer full at a time and write the entire modified buffer out at once.

Upvotes: 2

Joe
Joe

Reputation: 42597

You have two problems:

1) You should be checking that fread returns the number of items you requested, e.g. that you get a 1 back.

2) You should then be checking feof(fp), not comparing the character you read to EOF. This will tell you if your read returned less/no items because of EOF or some other reason.

Upvotes: 2

Related Questions