Re: ftell: two minor proposed patches

[Bruno Haible]

Hi Paul,

> 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;
>     }

I thought about this as well, but it has the drawback that "grep '(long)'"
wouldn't find this occurrence.

> 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));
> }

It's like the solution with the comment: Good readability, but "grep '(long)'"
won't find it. And surely the function should be 'static inline'.

Btw, when you rebased your proposed commits, the ChangeLog entries did not go
in at the top. Are you using the 'git-merge-changelog' driver? I think this
driver has the effect that even if on your side you had grouped several
ChangeLog entries under one date line, after the rebase your entries go at the
top. If not, there must be a bug in git-merge-changelog or in the way it's
installed...

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