Skip to content

Commit

Permalink
Improve error handling in BroadcastTimeDefs::string_to_date() (#745)
Browse files Browse the repository at this point in the history
* Improve the error handling and/or recovery of BroadcastTimeDefs::string_to_date().

* Fix tests.

* Temove trivial white space.

* Fix review comments.
  • Loading branch information
bakerstu authored Nov 11, 2023
1 parent e9e660a commit 05507d0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 7 deletions.
23 changes: 22 additions & 1 deletion src/openlcb/BroadcastTimeDefs.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,32 @@ int16_t BroadcastTimeDefs::string_to_rate_quarters(const std::string &srate)
bool BroadcastTimeDefs::string_to_date(
const std::string &sdate, int *year, int *month, int *day)
{
struct tm tm = {};
struct tm tm;
memset(&tm, 0, sizeof(tm));
if (strptime(sdate.c_str(), "%b %e, %Y", &tm) == nullptr)
{
return false;
}

// newlib does not have the proper boundary checking for strptime().
// Therefore we use mktime() to determine if the time we have is really
// valid or not. In newlib, mktime() can actually correct some invalid
// struct tm values by making some educated guesses.
//
// While glibc does have proper boundary checking for strptime(), it
// can still use mktime() to correct some invalid struct tm values by
// making some educated guesses.
time_t t = mktime(&tm);

// newlib does not correctly set the errno value when mktime()
// encounters an error. Instead it "only" returns -1, which is technically
// a valid time. We are counting on the fact that we zeroed out the struct
// tm above, and subsequently -1 cannot be an expected result.
if (t == (time_t)-1)
{
return false;
}

if (tm.tm_year < (0 - 1900) || tm.tm_year > (4095 - 1900))
{
// Out of range for openlcb.
Expand Down
13 changes: 7 additions & 6 deletions src/openlcb/BroadcastTimeDefs.cxxtest
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,10 @@ TEST_F(BroadcastTimeDefsTest, string_to_date)
parse_date("Feb 29, 2024", 2024, 2, 29);

// Not leap year
parse_date("Feb 29, 2023", 2023, 2, 29);
// No canonicalization in glibc
parse_date("Feb 31, 2023", 2023, 2, 31);
parse_date("Feb 29, 2023", 2023, 3, 1);

// canonize
parse_date("Feb 31, 2023", 2023, 3, 3);
}

TEST_F(BroadcastTimeDefsTest, string_to_date_fail)
Expand Down Expand Up @@ -202,9 +203,9 @@ TEST_F(BroadcastTimeDefsTest, date_canonicalize)

EXPECT_FALSE(canon_date("Aug 12, 2023"));
EXPECT_TRUE(canon_date("Aug 12, \n\t 2023", "Aug 12, 2023"));
EXPECT_FALSE(canon_date("Feb 31, 2023"));
EXPECT_FALSE(canon_date("Feb 31, 23"));
EXPECT_FALSE(canon_date("Feb 31, 3023"));
EXPECT_TRUE(canon_date("Feb 31, 2023", "Mar 3, 2023"));
EXPECT_TRUE(canon_date("Feb 31, 23", "Mar 3, 23"));
EXPECT_TRUE(canon_date("Feb 31, 3023", "Mar 3, 3023"));

EXPECT_TRUE(canon_date("Aug 9, 2023", "Aug 9, 2023"));

Expand Down

0 comments on commit 05507d0

Please sign in to comment.