[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: ftell: two minor proposed patches

From: Paul Eggert
Subject: Re: ftell: two minor proposed patches
Date: Sun, 24 Jul 2011 09:59:57 -0700
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20110617 Thunderbird/3.1.11

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;
      errno = EOVERFLOW;
      return -1;

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]