fread Returns < 1 While Reading Record from Binary File

Question

I am trying to read in student records (SRECs) from a binary file. The file is sure to contain at l complete record (the previous run writes the file). On my output, I'm getting the "ERROR: Could not read record from file" statement twice for two records. The printf(Reading record, numR) at the top displays 3 times, so I know the while loop is entered 3 times when it should have reached eof.

Any ideas? Thanks in advance!

/*Create and populate all 4 lists if lname list exists from previous usage*/
     if((readL = fopen("last", "rb")) == NULL)
     {
        printf("\nNew file will be created after server terminates.\n");
        created = 1;
     }
     else
        {           
        created = 0;

/*Read in record from binary file*/
        while(!feof(readL))
        {
            numR++;         
printf("Reading record %d\n", numR);

            /*Create new temporary SREC*/
            newSREC = (SREC*)malloc(sizeof(SREC));
            newSREC2 = (SREC*)malloc(sizeof(SREC));
            newSREC3 = (SREC*)malloc(sizeof(SREC));
            newSREC4 = (SREC*)malloc(sizeof(SREC));

            /*Read in one record from file*/
            if(fread(newSREC, sizeof(SREC), 1, readL) < 1)
            {
                newSREC2 = newSREC;
                newSREC3 = newSREC;
                newSREC4 = newSREC;

                firstL = insert(newSREC, 1, firstL);
                firstF = insert(newSREC2, 2, firstF);
                firstS = insert(newSREC3, 3, firstS);
                firstG = insert(newSREC4, 4, firstG);
            }
            else
            {
                printf("ERROR: Could not read record from file.\n");
            }
        }

        fclose(readL);
    }

Answer 1

Surface problem

On the face of it, this code has the error detection reversed:

if (fread(newSREC, sizeof(SREC), 1, readL) < 1)
{
    newSREC2 = newSREC;
    newSREC3 = newSREC;
    newSREC4 = newSREC;

    firstL = insert(newSREC, 1, firstL);
    firstF = insert(newSREC2, 2, firstF);
    firstS = insert(newSREC3, 3, firstS);
    firstG = insert(newSREC4, 4, firstG);
}
else
{
    printf("ERROR: Could not read record from file.\n");
}

If a complete record is read, the return from fread() will be 1; otherwise it will be zero. You should be using:

if (fread(newSREC, sizeof(SREC), 1, readL) == 1)

---

Delving deeper

The loop should not be using feof() — see while (!feof(file)) is always wrong. You should be using something like:

SREC srec;
while ((fread(&srec, sizeof(srec), 1, readL) == 1)
{
    SREC *newSREC1 = malloc(sizeof(*newSREC1));
    SREC *newSREC2 = malloc(sizeof(*newSREC2));
    SREC *newSREC3 = malloc(sizeof(*newSREC3));
    SREC *newSREC4 = malloc(sizeof(*newSREC4));
    if (newSREC1 == 0 || newSREC2 == 0 || newSREC3 == 0 || newSREC4 == 0)
    {
        free(newSREC1);
        free(newSREC2);
        free(newSREC3);
        free(newSREC4);
        …report error…
        break;
    }
    *newSREC1 = srec;
    *newSREC2 = srec;
    *newSREC3 = srec;
    *newSREC4 = srec;

    firstL = insert(newSREC1, 1, firstL);
    firstF = insert(newSREC2, 2, firstF);
    firstS = insert(newSREC3, 3, firstS);
    firstG = insert(newSREC4, 4, firstG);
}

// Optional analysis of why the loop ended — feof() vs ferror() vs short read

The code leaks memory like there's never any shortage of memory, ever. Having allocated newSREC2 etc, it overwrites the pointers with pointers to newSREC1. The assignments should be:

*newSREC2 = *newSREC; // Original code — see above for revision

I note that you can't allocate all 4 records in an array because you almost certainly need to be able to free the 4 records independently. That leaves a lot of repetition in the code.

Answer 2

From fread man page :

fread() does not distinguish between end-of-file and error, and callers must use feof(3) and ferror(3) to determine which occurred.

Change your tests for something like :

while (fread(...) > 0)
    // ...

if (ferror(file))
    // an error happened

Answer 3

The error indicator for EOF is only set upon trying to read beyond.

Test for successful read where you read.

In your example, test for reading exactly 1 record.