Re: ftell: two minor proposed patches

[Paul Eggert]

On 07/24/11 03:53, Bruno Haible wrote:
> You call it a "wraparound" situation. But there is no arithmetic operation,
> just a cast. Isn't there a better word to denote this situation?

Sorry, I don't know of one.  A cast is an arithmetic operation, though,
even though it often has zero runtime cost (in this sense it is like
"n + 0"); so "overflow" and "wraparound" are not inappropriate words here.

> I would leave the
>    return (long)offset;
> statement as it is. The code does a conversion, and if the code is more
> explicit about it, the better. Otherwise, when reading the code, I keep
> wondering "what is this if statement good for?", and when searching for
> casts, I have no chance to find it with 'grep'.

I left the cast alone, but still, I don't like it.
It's better to avoid casts when possible, as they can mask
more-important errors, e.g., converting pointers to integers.
Also, conversions happen all the time in C code, and it would
overly clutter the code to mark each one with a cast.
Finally, for this particular case, surely it's obvious
what's going on even without a cast.

I thought of making it even more obvious
by adding a comment, like this:

  if (LONG_MIN <= offset && offset <= LONG_MAX)
    {
       /* Convert offset to the result type 'long', and return.  */
       return offset;
    }

but I dunno, that's uncomfortably close to:

  index++; /* Add one to index.  */

and clutter like this is best avoided.

How about the following idea instead?  It makes the conversion
clearer, and it avoids the cast.

static long
convert_off_t_to_long (off_t n)
{
  if (LONG_MIN <= n && n <= LONG_MAX)
    return n;
  else
    {
      errno = EOVERFLOW;
      return -1;
    }
}

long
ftell (FILE *fp)
{
  /* Use the replacement ftello function with all its workarounds.  */
  return convert_off_t_to_long (ftello (fp));
}