FW: [casting; overflow detection]

Robert Elz kre at munnari.OZ.AU
Sat Jan 15 04:43:55 UTC 2011


    Date:        Thu, 13 Jan 2011 10:52:44 -0800
    From:        Paul Eggert <eggert at cs.ucla.edu>
    Message-ID:  <4D2F49FC.8070601 at cs.ucla.edu>


  | @@ -598,15 +598,12 @@ comptm(atmp, btmp)
  |  register const struct tm * const atmp;
  |  register const struct tm * const btmp;
  |  {
  | -	register int	result;
  | -
  | -	if ((result = (atmp->tm_year - btmp->tm_year)) == 0 &&
  | -		(result = (atmp->tm_mon - btmp->tm_mon)) == 0 &&
  | -		(result = (atmp->tm_mday - btmp->tm_mday)) == 0 &&
  | -		(result = (atmp->tm_hour - btmp->tm_hour)) == 0 &&
  | -		(result = (atmp->tm_min - btmp->tm_min)) == 0)
  | -			result = atmp->tm_sec - btmp->tm_sec;
  | -	return result;
  | +	return atmp->tm_year != btmp->tm_year
  | +	    || atmp->tm_mon  != btmp->tm_mon
  | +	    || atmp->tm_mday != btmp->tm_mday
  | +	    || atmp->tm_hour != btmp->tm_hour
  | +	    || atmp->tm_min  != btmp->tm_min
  | +	    || atmp->tm_sec  != btmp->tm_sec;

If you're going to make that change, which is OK, as date.c doesn't
need more than that, then you should probably also rename the function
to make it clear that it is now an equality test for struct tm's, rather
than a comparison of struct tm's which it was before (but which for its
usage in date.c it did not need to be).

You don't get to make the same kind of change to the very similar function in 
localtime.c though ...

Most of it is also not needed, the struct tm's are normalised, which means
that "atmp->tm_hour - btmp->tm_hour" cannot possibly underflow or overflow,
the values cannot possibly be big enough (in magitude) - so we don't really
need to take extremes to protect against overflow that the raw data types
potentially would allow.

  |  	number0 = *number;
  | +	if (delta < 0 ? number0 < delta - INT_MIN : INT_MAX - delta < number0)
  | +		  return 1;

Surely the first test there should be
		number0 < INT_MIN - delta
?

INT_MIN is a big (in magnitude) negative number, sutracting that from a
relatively small (negative) number must generate a quite large positive
number, almost any value for number0 is going to be smaller than that,
leading to a false detection of underflow.


  |  	number0 = *number;
  | +	if (delta < 0 ? number0 < delta - LONG_MIN : LONG_MAX - delta < number0)
  | +		  return 1;

Ditto.


I haven't checked the proposed mods to zic or zdump to see whether
they're correct - in general I don't much llike pandering to this
nonsense from the C standards people.   The chances that C will ever
be used in any meaningful way on any hardware where interger overflow
doesn't wrap is close to 0 (just because all the existing software is
written with that assumption, and no sane manufacturer is ever going to
burden themselves with needing to fix all of it).   So whatever the
academic possibilities are for this (and some other - such as not assuming
2's complement for negative numbers - again, today no other choice is
possible) the practical environment is that they're never going to happen,
and what the standards should deal with is what we actually see in practice,
not might be possible in the fevered imagination of some theoretician.

kre




More information about the tz mailing list