A blog post about a bug found by PVS-Studio in Chromium:
The C++ code that contains the bug:
static const int kDaysInMonth[13] = {
0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
};
bool ValidateDateTime(const DateTime& time) {
if (time.year < 1 || time.year > 9999 ||
time.month < 1 || time.month > 12 ||
time.day < 1 || time.day > 31 ||
time.hour < 0 || time.hour > 23 ||
time.minute < 0 || time.minute > 59 ||
time.second < 0 || time.second > 59) {
return false;
}
if (time.month == 2 && IsLeapYear(time.year)) {
return time.month <= kDaysInMonth[time.month] + 1;
} else {
return time.month <= kDaysInMonth[time.month];
}
}
At the end of the blog post Andrey Karpov asks: "How can we improve our style to ward off errors like that?"
My answer is to write Rust-like code like this:
type Year = u16[1 ... 9_999];
type Month = u8[1 ... 12];
type Day = u8[1 ... 31];
type Hour = u8[0 ... 23];
type Minute = u8[0 ... 59];
type Second = u8[0 ... 59];
#[only_impl_new]
struct DateTime {
year: Year,
month: Month,
day: Day,
hour: Hour,
minute: Minute,
second: Second,
}
impl Year {
fn is_leap_year(&self) -> bool {
// implementation ...
true
}
}
impl DateTime {
const DAYS_IN_MONTH: [Day; Month] =
[31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31];
fn new(year: Year,
month: Month,
day: Day,
hour: Hour,
minute: Minute,
second: Second) -> Option<Self> {
// This leap is inferred of type Day.
let leap = if month == 2 && year.is_leap_year() { 1 } else { 0 };
if time.day <= Self::DAYS_IN_MONTH[month] + leap {
Some(DateTime(year, month, day, hour, minute, second))
} else {
None
}
}
}
It shows some ideas:
- This code is untested and it's meant just to show rough ideas, its syntaxes are far from final;
- Newtypes like Year are ranged, they can't be built with values outside their range (the compiler adds run-time or compile-time tests to verify their range, so you don't need to verify their range manually);
- #[only_impl_new] means that you can build a DateTime only through the new() function (this can be done today in Rust using privacy and modules, but it requires more boilerplate);
- is_leap_year is a method;
- No need to validate a DateTime, its correctness is verified when you build it;
- All types are strong so you can't mismatch them as in the blog post;
- DAYS_IN_MONTH is an array (it's not an associative array), it contains values of type Day and it's indexed by indices of type Month. There's little space for bugs here;
- Year is still always > 0, so you need a different time data structure if you want to manage historical times...