Should I check the return value of fseek() when calculating the length of a file?

1.8k Views Asked by At

I have this idiomatic snippet for getting the length of a binary file:

    fseek(my_file, 0, SEEK_END);
    const size_t file_size = ftell(my_file);

…I know, to be pedantic fseek(file, 0, SEEK_END) has undefined behavior for a binary stream [1] – but frankly on the platforms where this is a problem I also don't have fstat() and anyway this is a topic for another question…

My question is: Should I check the return value of fseek() in this case?

    if (fseek(my_file, 0, SEEK_END)) {

        return 1;

    }

    const size_t file_size = ftell(my_file);

I have never seen fseek() been checked in a case like this, and I also wonder what kind of error fseek() could possibly ever return here.

EDIT:

After reading Clifford's answer, I also think that the best way to deal with fseek() and ftell() return values while calculating the size of a file is to write a dedicated function. However Clifford's good suggestion could not deal with the size_t data type (we need a size after all!), so I guess that the most practical approach in the end would be to use a pointer for storing the size of the file, and keep the return value of our dedicated function only for failures. Here is my contribution to Clifford's solution for a safe size calculator:

int fsize (FILE * const file, size_t * const size) {

    long int ftell_retval;

    if (fseek(file, 0, SEEK_END) || (ftell_retval = ftell(file)) < 0) {

        /*  Error  */
        *size = 0;
        return 1;

    }

    *size = (size_t) ftell_retval;
    return 0;

}

So that when we need to know the length of a file we could simply do:

size_t file_size;

if (fsize(my_file, &file_size)) {

    fprintf(stderr, "Error calculating the length of the file\n");
    return 1;

}
4

There are 4 best solutions below

12
On BEST ANSWER

You need perhaps to ask yourself two questions:

  1. What will ftell() return if fseek() has failed?
  2. Can I handle failure in any meaningful way?

If fseek() fails it returns a non-zero value. If ftell() fails (which it likely will if fseek() has failed), it will return -1L - so is more deterministic, which from an error handling point of view is better.

However there are potentially ways in which fseek() could fail that do not cause ftell() to fail (unlikely perhaps, but the failure modes are implementation defined), so it is better perhaps to test fseek() to be sure you are not getting an erroneous answer from ftell().

Since your aim is to get the file size, and the use of fseek/ftell is just a way of synthesising that, it makes more sense to define a file-size function, so that the caller need only be concerned with handling the failure to obtain a valid file size rather than the failure of implementation details. The point being is if you want the file size, you don't want to have to handle errors for fseek() since that was a means to an end and not directly related to what you need to achieve - failure of fseek() is a non-deterministic side-effect, and the effect is an unknown file size - better then to behave "as-if" ftell() had failed without risking misleading behaviour by actually calling ftell():

long fsize( FILE* file )
{
    long size = -1 '  // as-if ftell() had failed
    if( fseek( file, 0, SEEK_END ) == 0 )
    {
        size = ftell( file ) ;
    }

    return size ;
}

Then your code will be:

const long file_size = fsize(my_file);

Then at the application level you only need to handle the error file_size < 0, you have no interest in whether fseek() or ftell() failed, just that you don't know the file size.

10
On

Yes, check return value, yet be more careful with type changes.

Note the the range of size_t may be more or less than 0...LONG_MAX.

// function returns an error flag
int fsize (FILE * file, size_t *size) {
  if (fseek(file, 0, SEEK_END)) {
    return 1; // fseek error  
  }

  long ftell_retval = ftell(file);
  if (ftell_retval == -1) {
    return 1; // ftell error
  }

  // Test if the file size fits in a `size_t`.
  // Improved type conversions here.
  // Portably *no* overflow possible.
  if (ftell_retval < 0 || (unsigned long) ftell_retval > SIZE_MAX) {
    return 1; // range error
  }

  *size = (size_t) ftell_retval;
  return 0;
}

Portability

Direct conversion of a long to size_t and vice versa is portably challenging given the relationship of LONG_MAX, SIZE_MAX is not defined. It may be <,==, >.

Instead first test for < 0, then, if positive, convert to unsigned long. C specifies that LONG_MAX <= ULONG_MAX, so we are OK here. Then compare the unsigned long to SIZE_MAX. Since both types are some unsigned type, the compare simply converts to the wider of the two. Again no range loss.

0
On

Its always a good practice to test the return value of a function and handle it on time otherwise strange behaviour might occur which you won't be able to understand or find without an exhaustive debugging.

You can read the following link about the return value of fseek: fseek in the return value section.

This if statement is neglectable in the code pipeline while make its easier to treat problems when it occur.

0
On

fseek can return an error in the case where the file handle is a pipe (or a serial stream).
At that point, ftell can't even tell you where it's at, because in those circumstances it's more "wherever you go, there you are".