Gary Thomas wrote:
The _simple_mktime() function could hang (*), given bad input data
and no asserts. This was found in the real world - a brand
new I2C clock which had never been initialized passed in an
incorrect month and the whole thing locked up :-(
Arguably if a wallclock driver knows that it's possible for the hardware to
express bogus dates, it should be sanitizing its output (for many
wallclocks, the restrictions on possible values is intrinsic of course, so
this isn't universally applicable). Hence asserts rather than runtime
check, and also hence why simple_mktime doesn't have an error return.
--- io/wallclock/current/include/wallclock/wallclock.inl 27 Mar 2003 08:38:07 -0000 1.7
+++ io/wallclock/current/include/wallclock/wallclock.inl 9 Sep 2008 11:30:19 -0000
@@ -74,26 +74,26 @@ static time_t
_simple_mktime(cyg_uint32 year, cyg_uint32 mon,
cyg_uint32 day, cyg_uint32 hour,
cyg_uint32 min, cyg_uint32 sec)
{
time_t secs;
- cyg_uint32 y, m, days;
+ cyg_int32 y, m, days;
CYG_ASSERT(year <= 3124, "Year is unreasonably large");
CYG_ASSERT(mon <= 12, "Month is invalid");
CYG_ASSERT(day <= 31, "Day is invalid");
CYG_ASSERT(hour <= 23, "Hour is invalid");
CYG_ASSERT(min <= 59, "Minutes is invalid");
CYG_ASSERT(sec <= 61, "Seconds is invalid");
// Number of days due to years
days = 0;
- for (y = 1970; y < year; y++)
+ for (y = 1970; y < (cyg_int32)year; y++)
days += year_days(y);
// Due to months
- for (m = 0; m < mon-1; m++)
+ for (m = 0; m < (cyg_int32)mon-1; m++)
days += days_per_month[is_leap(year)][m];
// Add days
days += day - 1;
// Add hours, minutes, and seconds
If a device can return bogus values, then year==0 or mon==0 aren't the only
potential problems - for a device which would initially come up with a
random (rather than 0) value, it would be just as problematic as that case.
For example the above could easily walk off the end of the days_per_month
array.
I guess I mean the above change isn't a big deal, but doesn't really
address the main problem - wallclock drivers should validate data they read
if that's relevant for a particular device.