Re: ftell: two minor proposed patches

[Bruno Haible]

Hi Paul,

Good points on both.

> @@ -28,8 +29,8 @@ ftell (FILE *fp)
>  {
>    /* Use the replacement ftello function with all its workarounds.  */
>    off_t offset = ftello (fp);
> -  if (offset == (long)offset)
> -    return (long)offset;
> +  if (LONG_MIN <= offset && offset <= LONG_MAX)
> +    return offset;

Indeed, ISO C 99 6.3.1.3.(3) says
  "Otherwise, the new type is signed and the value cannot be represented
   in it; either the result is implementation-defined or an implementation-
   defined signal is raised."

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?

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'.

> +     ftell: don't include <unistd.h>
> +     * lib/ftell.c: Don't include <unistd.h>.  <stdio.h> is now
> +     guaranteed to define off_t, since the ftell module depends on the
> +     stdio module.

Yes, right. I would change s/since/and/.

Bruno
-- 
In memoriam Ezechiele Ramin <http://en.wikipedia.org/wiki/Ezechiele_Ramin>