Re: Help needed on #95010

[Phil Carmody <fatphil@altavista.com>]

On Mon, 25 June 2001, Craig Small wrote:
> On Tue, Jun 19, 2001 at 03:13:31PM +0200, Samuel Tardieu wrote:
> > could one of you have a look at 95010? I cannot find an Alpha machine
> > with Debian and X installed (vic needs X11 headers) to debug that
> > (this is probably a minor problem, or 64 bits machines will be removed
> > from vic Architecture line).
> It is and it isn't, here is a patch.
> 
> The problems
>   configure was not defining HAVE_INET_NTOP and HAVE_INET_PTON
>   time() is in <time.h>
>   if (<some-int> == <some-long>) is dumb as long != int, casting added
>   to provide sanity

Sanity's good. Extra pairs of eyes are sometimes handy too. I've proposed a different modification below, which I believe should be more sane.

> diff -ur vic-2.8ucl1.1.3.orig/common/mbus_parser.c vic-2.8ucl1.1.3/common/mbus_parser.c
> --- vic-2.8ucl1.1.3.orig/common/mbus_parser.c    Thu Feb 17 23:59:36 2000
> +++ vic-2.8ucl1.1.3/common/mbus_parser.c    Tue Jun 26 13:17:03 2001
> @@ -167,7 +167,7 @@
>          }
>  
>      *i = strtol(m->buffer, &p, 10);
> -    if (((*i == LONG_MAX) || (*i == LONG_MIN)) && (errno == ERANGE)) {
> +    if (((*i == (int)LONG_MAX) || (*i == (int)LONG_MIN)) && (errno == ERANGE)) {
>          debug_msg("integer out of range\n");
>          return FALSE;
>      }

If the problem is that you are trying to read a signed integer into *i using the strtol functions, then the range error is the condition 
  long-value-read<INT_MIN || long-value-read>INT_MAX
For example, if the string being read contains the value 999999999999, then it is _not_ out of range for strtol, and errno will not be set. However, *i will be set to 9999999999l%~0u, i.e. a bogus number. The error condition inside strtol is flagged by both errno and the extremal return values, and thus can be checked by only using errno.

I would propose

     long tmpLong;
     /* ... */
     tmpLong = strtol(m->buffer, &p, 10);
     if((tmpLong < (long)INT_MIN) ||
        (tmpLong > (long)INT_MAX) ||
        (errno == ERANGE)) {
         debug_msg("integer out of range\n");
         *i = /* whatever makes sense if anything */
         return FALSE;
     } else {
         *i = (int)tmpLong;
     }

This is based not on knowledge of the clients of the above code, but merely 'linting by eye' using nothing but static heuristics.
When sizeof(int)==sizeof(long), the above code isn't quite equivalent to the original, as the boundary conditions are never satisfiable, and may cause compiler heuristics. In which case
     if(tmpLong == (long)(int)tmpLong)
could be used as the 'does it fit in an int' condition. Either way, keeping the value as a long until you've checked it is the safer thing to do.


Phil

Mathematics should not have to involve martyrdom;
Support Eric Weisstein, see http://mathworld.wolfram.com
Find the best deals on the web at AltaVista Shopping!
http://www.shopping.altavista.com