[libc-commits] [libc] change the return value type of mktime_internal to time_t (PR #132231)
via libc-commits
libc-commits at lists.llvm.org
Sun Mar 23 07:02:19 PDT 2025
https://github.com/wsehjk updated https://github.com/llvm/llvm-project/pull/132231
>From 63c534280a0148d3c0f37806fd93726c51652e36 Mon Sep 17 00:00:00 2001
From: wang shiwen <Wse1714401046 at gmail.com>
Date: Thu, 20 Mar 2025 23:03:18 +0800
Subject: [PATCH 1/5] change the return value type of mktime_internal to time_t
---
libc/src/time/mktime.cpp | 7 +++---
libc/src/time/time_utils.cpp | 46 +++++++++++++++++++++++-------------
libc/src/time/time_utils.h | 9 +++----
3 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/libc/src/time/mktime.cpp b/libc/src/time/mktime.cpp
index fc05ff2930434..4c035a15bfe23 100644
--- a/libc/src/time/mktime.cpp
+++ b/libc/src/time/mktime.cpp
@@ -15,13 +15,14 @@
namespace LIBC_NAMESPACE_DECL {
LLVM_LIBC_FUNCTION(time_t, mktime, (struct tm * tm_out)) {
- int64_t seconds = time_utils::mktime_internal(tm_out);
+ bool out_of_range_flag = true;
+ time_t seconds = time_utils::mktime_internal(tm_out, &out_of_range_flag);
// Update the tm structure's year, month, day, etc. from seconds.
- if (time_utils::update_from_seconds(seconds, tm_out) < 0)
+ if (out_of_range_flag || time_utils::update_from_seconds(seconds, tm_out) < 0)
return time_utils::out_of_range();
- return static_cast<time_t>(seconds);
+ return seconds;
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/time/time_utils.cpp b/libc/src/time/time_utils.cpp
index 3ccb2dd934967..f4f06e276abde 100644
--- a/libc/src/time/time_utils.cpp
+++ b/libc/src/time/time_utils.cpp
@@ -18,7 +18,7 @@ namespace LIBC_NAMESPACE_DECL {
namespace time_utils {
// TODO: clean this up in a followup patch
-int64_t mktime_internal(const tm *tm_out) {
+time_t mktime_internal(const tm *tm_out, bool* out_of_range_flag) {
// Unlike most C Library functions, mktime doesn't just die on bad input.
// TODO(rtenneti); Handle leap seconds.
int64_t tm_year_from_base = tm_out->tm_year + time_constants::TIME_YEAR_BASE;
@@ -26,21 +26,33 @@ int64_t mktime_internal(const tm *tm_out) {
// 32-bit end-of-the-world is 03:14:07 UTC on 19 January 2038.
if (sizeof(time_t) == 4 &&
tm_year_from_base >= time_constants::END_OF32_BIT_EPOCH_YEAR) {
- if (tm_year_from_base > time_constants::END_OF32_BIT_EPOCH_YEAR)
- return time_utils::out_of_range();
- if (tm_out->tm_mon > 0)
- return time_utils::out_of_range();
- if (tm_out->tm_mday > 19)
- return time_utils::out_of_range();
+ if (tm_year_from_base > time_constants::END_OF32_BIT_EPOCH_YEAR) {
+ *out_of_range_flag = true;
+ return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ }
+ if (tm_out->tm_mon > 0) {
+ *out_of_range_flag = true;
+ return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ }
+ if (tm_out->tm_mday > 19) {
+ *out_of_range_flag = true;
+ return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ }
else if (tm_out->tm_mday == 19) {
- if (tm_out->tm_hour > 3)
- return time_utils::out_of_range();
+ if (tm_out->tm_hour > 3) {
+ *out_of_range_flag = true;
+ return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ }
else if (tm_out->tm_hour == 3) {
- if (tm_out->tm_min > 14)
- return time_utils::out_of_range();
+ if (tm_out->tm_min > 14) {
+ *out_of_range_flag = true;
+ return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ }
else if (tm_out->tm_min == 14) {
- if (tm_out->tm_sec > 7)
- return time_utils::out_of_range();
+ if (tm_out->tm_sec > 7) {
+ *out_of_range_flag = true;
+ return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ }
}
}
}
@@ -66,7 +78,7 @@ int64_t mktime_internal(const tm *tm_out) {
bool tm_year_is_leap = time_utils::is_leap_year(tm_year_from_base);
// Calculate total number of days based on the month and the day (tm_mday).
- int64_t total_days = tm_out->tm_mday - 1;
+ int total_days = tm_out->tm_mday - 1;
for (int64_t i = 0; i < month; ++i)
total_days += time_constants::NON_LEAP_YEAR_DAYS_IN_MONTH[i];
// Add one day if it is a leap year and the month is after February.
@@ -102,7 +114,7 @@ int64_t mktime_internal(const tm *tm_out) {
// TODO: https://github.com/llvm/llvm-project/issues/121962
// Need to handle timezone and update of tm_isdst.
- int64_t seconds = tm_out->tm_sec +
+ time_t seconds = tm_out->tm_sec +
tm_out->tm_min * time_constants::SECONDS_PER_MIN +
tm_out->tm_hour * time_constants::SECONDS_PER_HOUR +
total_days * time_constants::SECONDS_PER_DAY;
@@ -136,7 +148,7 @@ static int64_t computeRemainingYears(int64_t daysPerYears,
//
// Compute the number of months from the remaining days. Finally, adjust years
// to be 1900 and months to be from January.
-int64_t update_from_seconds(int64_t total_seconds, tm *tm) {
+int64_t update_from_seconds(time_t total_seconds, tm *tm) {
// Days in month starting from March in the year 2000.
static const char daysInMonth[] = {31 /* Mar */, 30, 31, 30, 31, 31,
30, 31, 30, 31, 31, 29};
@@ -152,7 +164,7 @@ int64_t update_from_seconds(int64_t total_seconds, tm *tm) {
: INT_MAX * static_cast<int64_t>(
time_constants::NUMBER_OF_SECONDS_IN_LEAP_YEAR);
- time_t ts = static_cast<time_t>(total_seconds);
+ time_t ts = total_seconds;
if (ts < time_min || ts > time_max)
return time_utils::out_of_range();
diff --git a/libc/src/time/time_utils.h b/libc/src/time/time_utils.h
index 68eaac8c04f11..ac5d48815e698 100644
--- a/libc/src/time/time_utils.h
+++ b/libc/src/time/time_utils.h
@@ -26,11 +26,11 @@ namespace time_utils {
// calculates the seconds from the epoch for tm_in. Does not update the struct,
// you must call update_from_seconds for that.
-int64_t mktime_internal(const tm *tm_out);
+time_t mktime_internal(const tm *tm_out, bool* out_of_range_flag);
// Update the "tm" structure's year, month, etc. members from seconds.
// "total_seconds" is the number of seconds since January 1st, 1970.
-int64_t update_from_seconds(int64_t total_seconds, tm *tm);
+int64_t update_from_seconds(time_t total_seconds, tm *tm);
// TODO(michaelrj): move these functions to use ErrorOr instead of setting
// errno. They always accompany a specific return value so we only need the one
@@ -84,7 +84,7 @@ LIBC_INLINE char *asctime(const tm *timeptr, char *buffer,
}
LIBC_INLINE tm *gmtime_internal(const time_t *timer, tm *result) {
- int64_t seconds = *timer;
+ time_t seconds = *timer;
// Update the tm structure's year, month, day, etc. from seconds.
if (update_from_seconds(seconds, result) < 0) {
out_of_range();
@@ -329,7 +329,8 @@ class TMReader final {
}
LIBC_INLINE time_t get_epoch() const {
- return static_cast<time_t>(mktime_internal(timeptr));
+ bool out_of_range_flag = true;
+ return mktime_internal(timeptr, &out_of_range_flag);
}
// returns the timezone offset in microwave time:
>From f973ff93b2fa5cee7a96ec261f1b6e5a7baec88b Mon Sep 17 00:00:00 2001
From: wang shiwen <Wse1714401046 at gmail.com>
Date: Thu, 20 Mar 2025 23:08:14 +0800
Subject: [PATCH 2/5] intialize out_of_range_flag to be false
---
libc/src/time/mktime.cpp | 2 +-
libc/src/time/time_utils.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libc/src/time/mktime.cpp b/libc/src/time/mktime.cpp
index 4c035a15bfe23..42c93a6c7f9d8 100644
--- a/libc/src/time/mktime.cpp
+++ b/libc/src/time/mktime.cpp
@@ -15,7 +15,7 @@
namespace LIBC_NAMESPACE_DECL {
LLVM_LIBC_FUNCTION(time_t, mktime, (struct tm * tm_out)) {
- bool out_of_range_flag = true;
+ bool out_of_range_flag = false;
time_t seconds = time_utils::mktime_internal(tm_out, &out_of_range_flag);
// Update the tm structure's year, month, day, etc. from seconds.
diff --git a/libc/src/time/time_utils.h b/libc/src/time/time_utils.h
index ac5d48815e698..6081f250d1ffe 100644
--- a/libc/src/time/time_utils.h
+++ b/libc/src/time/time_utils.h
@@ -329,7 +329,7 @@ class TMReader final {
}
LIBC_INLINE time_t get_epoch() const {
- bool out_of_range_flag = true;
+ bool out_of_range_flag = false;
return mktime_internal(timeptr, &out_of_range_flag);
}
>From a914e1b7f7a84c74a816ff953317a5cb12cc04c3 Mon Sep 17 00:00:00 2001
From: wang shiwen <Wse1714401046 at gmail.com>
Date: Fri, 21 Mar 2025 19:32:03 +0800
Subject: [PATCH 3/5] use struct as return value type
---
libc/src/time/mktime.cpp | 3 +--
libc/src/time/time_utils.cpp | 39 ++++++++++++++----------------------
libc/src/time/time_utils.h | 12 ++++++++---
3 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/libc/src/time/mktime.cpp b/libc/src/time/mktime.cpp
index 42c93a6c7f9d8..7532116d57084 100644
--- a/libc/src/time/mktime.cpp
+++ b/libc/src/time/mktime.cpp
@@ -15,8 +15,7 @@
namespace LIBC_NAMESPACE_DECL {
LLVM_LIBC_FUNCTION(time_t, mktime, (struct tm * tm_out)) {
- bool out_of_range_flag = false;
- time_t seconds = time_utils::mktime_internal(tm_out, &out_of_range_flag);
+ auto [seconds, out_of_range_flag] = time_utils::mktime_internal(tm_out);
// Update the tm structure's year, month, day, etc. from seconds.
if (out_of_range_flag || time_utils::update_from_seconds(seconds, tm_out) < 0)
diff --git a/libc/src/time/time_utils.cpp b/libc/src/time/time_utils.cpp
index f4f06e276abde..4ca802f28ae1c 100644
--- a/libc/src/time/time_utils.cpp
+++ b/libc/src/time/time_utils.cpp
@@ -18,7 +18,7 @@ namespace LIBC_NAMESPACE_DECL {
namespace time_utils {
// TODO: clean this up in a followup patch
-time_t mktime_internal(const tm *tm_out, bool* out_of_range_flag) {
+TMParse mktime_internal(const tm *tm_out) {
// Unlike most C Library functions, mktime doesn't just die on bad input.
// TODO(rtenneti); Handle leap seconds.
int64_t tm_year_from_base = tm_out->tm_year + time_constants::TIME_YEAR_BASE;
@@ -27,31 +27,22 @@ time_t mktime_internal(const tm *tm_out, bool* out_of_range_flag) {
if (sizeof(time_t) == 4 &&
tm_year_from_base >= time_constants::END_OF32_BIT_EPOCH_YEAR) {
if (tm_year_from_base > time_constants::END_OF32_BIT_EPOCH_YEAR) {
- *out_of_range_flag = true;
- return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
}
if (tm_out->tm_mon > 0) {
- *out_of_range_flag = true;
- return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
}
if (tm_out->tm_mday > 19) {
- *out_of_range_flag = true;
- return time_constants::OUT_OF_RANGE_RETURN_VALUE;
- }
- else if (tm_out->tm_mday == 19) {
+ return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
+ } else if (tm_out->tm_mday == 19) {
if (tm_out->tm_hour > 3) {
- *out_of_range_flag = true;
- return time_constants::OUT_OF_RANGE_RETURN_VALUE;
- }
- else if (tm_out->tm_hour == 3) {
+ return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
+ } else if (tm_out->tm_hour == 3) {
if (tm_out->tm_min > 14) {
- *out_of_range_flag = true;
- return time_constants::OUT_OF_RANGE_RETURN_VALUE;
- }
- else if (tm_out->tm_min == 14) {
+ return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
+ } else if (tm_out->tm_min == 14) {
if (tm_out->tm_sec > 7) {
- *out_of_range_flag = true;
- return time_constants::OUT_OF_RANGE_RETURN_VALUE;
+ return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
}
}
}
@@ -78,7 +69,7 @@ time_t mktime_internal(const tm *tm_out, bool* out_of_range_flag) {
bool tm_year_is_leap = time_utils::is_leap_year(tm_year_from_base);
// Calculate total number of days based on the month and the day (tm_mday).
- int total_days = tm_out->tm_mday - 1;
+ int64_t total_days = tm_out->tm_mday - 1;
for (int64_t i = 0; i < month; ++i)
total_days += time_constants::NON_LEAP_YEAR_DAYS_IN_MONTH[i];
// Add one day if it is a leap year and the month is after February.
@@ -115,10 +106,10 @@ time_t mktime_internal(const tm *tm_out, bool* out_of_range_flag) {
// TODO: https://github.com/llvm/llvm-project/issues/121962
// Need to handle timezone and update of tm_isdst.
time_t seconds = tm_out->tm_sec +
- tm_out->tm_min * time_constants::SECONDS_PER_MIN +
- tm_out->tm_hour * time_constants::SECONDS_PER_HOUR +
- total_days * time_constants::SECONDS_PER_DAY;
- return seconds;
+ tm_out->tm_min * time_constants::SECONDS_PER_MIN +
+ tm_out->tm_hour * time_constants::SECONDS_PER_HOUR +
+ total_days * time_constants::SECONDS_PER_DAY;
+ return {seconds, false};
}
static int64_t computeRemainingYears(int64_t daysPerYears,
diff --git a/libc/src/time/time_utils.h b/libc/src/time/time_utils.h
index 6081f250d1ffe..fb8058f0e72b6 100644
--- a/libc/src/time/time_utils.h
+++ b/libc/src/time/time_utils.h
@@ -24,9 +24,15 @@
namespace LIBC_NAMESPACE_DECL {
namespace time_utils {
+
+// store the result of mktime_internal(const tm* )
+struct TMParse {
+ time_t seconds;
+ bool out_of_range_flag;
+};
// calculates the seconds from the epoch for tm_in. Does not update the struct,
// you must call update_from_seconds for that.
-time_t mktime_internal(const tm *tm_out, bool* out_of_range_flag);
+TMParse mktime_internal(const tm *tm_out);
// Update the "tm" structure's year, month, etc. members from seconds.
// "total_seconds" is the number of seconds since January 1st, 1970.
@@ -329,8 +335,8 @@ class TMReader final {
}
LIBC_INLINE time_t get_epoch() const {
- bool out_of_range_flag = false;
- return mktime_internal(timeptr, &out_of_range_flag);
+ auto[seconds, _] = mktime_internal(timeptr);
+ return seconds;
}
// returns the timezone offset in microwave time:
>From 8f53621e3ccd04b1c503ac400f137b0a673999b7 Mon Sep 17 00:00:00 2001
From: wang shiwen <Wse1714401046 at gmail.com>
Date: Fri, 21 Mar 2025 19:35:05 +0800
Subject: [PATCH 4/5] remove unnecessary variable
---
libc/src/time/time_utils.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libc/src/time/time_utils.cpp b/libc/src/time/time_utils.cpp
index 4ca802f28ae1c..0cb080fc3aa4f 100644
--- a/libc/src/time/time_utils.cpp
+++ b/libc/src/time/time_utils.cpp
@@ -155,8 +155,7 @@ int64_t update_from_seconds(time_t total_seconds, tm *tm) {
: INT_MAX * static_cast<int64_t>(
time_constants::NUMBER_OF_SECONDS_IN_LEAP_YEAR);
- time_t ts = total_seconds;
- if (ts < time_min || ts > time_max)
+ if (total_seconds < time_min || total_seconds > time_max)
return time_utils::out_of_range();
int64_t seconds =
>From f334ee5cba2b040ca56d85a496fd3dbf783ee1e9 Mon Sep 17 00:00:00 2001
From: wang shiwen <Wse1714401046 at gmail.com>
Date: Sun, 23 Mar 2025 22:02:21 +0800
Subject: [PATCH 5/5] use cpp::optional
---
libc/src/time/mktime.cpp | 11 ++++++++---
libc/src/time/time_utils.cpp | 22 ++++++++++------------
libc/src/time/time_utils.h | 15 ++++++---------
3 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/libc/src/time/mktime.cpp b/libc/src/time/mktime.cpp
index 7532116d57084..8d40956eb784a 100644
--- a/libc/src/time/mktime.cpp
+++ b/libc/src/time/mktime.cpp
@@ -15,10 +15,15 @@
namespace LIBC_NAMESPACE_DECL {
LLVM_LIBC_FUNCTION(time_t, mktime, (struct tm * tm_out)) {
- auto [seconds, out_of_range_flag] = time_utils::mktime_internal(tm_out);
-
+ auto mktime_result = time_utils::mktime_internal(tm_out);
+ time_t seconds;
+ if (mktime_result.has_value()) {
+ return time_utils::out_of_range();
+ } else {
+ seconds = mktime_result.value();
+ }
// Update the tm structure's year, month, day, etc. from seconds.
- if (out_of_range_flag || time_utils::update_from_seconds(seconds, tm_out) < 0)
+ if (time_utils::update_from_seconds(seconds, tm_out) < 0)
return time_utils::out_of_range();
return seconds;
diff --git a/libc/src/time/time_utils.cpp b/libc/src/time/time_utils.cpp
index 0cb080fc3aa4f..48bc1a999e30e 100644
--- a/libc/src/time/time_utils.cpp
+++ b/libc/src/time/time_utils.cpp
@@ -18,7 +18,7 @@ namespace LIBC_NAMESPACE_DECL {
namespace time_utils {
// TODO: clean this up in a followup patch
-TMParse mktime_internal(const tm *tm_out) {
+cpp::optional<time_t> mktime_internal(const tm *tm_out) {
// Unlike most C Library functions, mktime doesn't just die on bad input.
// TODO(rtenneti); Handle leap seconds.
int64_t tm_year_from_base = tm_out->tm_year + time_constants::TIME_YEAR_BASE;
@@ -26,23 +26,21 @@ TMParse mktime_internal(const tm *tm_out) {
// 32-bit end-of-the-world is 03:14:07 UTC on 19 January 2038.
if (sizeof(time_t) == 4 &&
tm_year_from_base >= time_constants::END_OF32_BIT_EPOCH_YEAR) {
- if (tm_year_from_base > time_constants::END_OF32_BIT_EPOCH_YEAR) {
- return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
- }
- if (tm_out->tm_mon > 0) {
- return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
- }
+ if (tm_year_from_base > time_constants::END_OF32_BIT_EPOCH_YEAR)
+ return {};
+ if (tm_out->tm_mon > 0)
+ return {};
if (tm_out->tm_mday > 19) {
- return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
+ return {};
} else if (tm_out->tm_mday == 19) {
if (tm_out->tm_hour > 3) {
- return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
+ return {};
} else if (tm_out->tm_hour == 3) {
if (tm_out->tm_min > 14) {
- return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
+ return {};
} else if (tm_out->tm_min == 14) {
if (tm_out->tm_sec > 7) {
- return {time_constants::OUT_OF_RANGE_RETURN_VALUE, true};
+ return {};
}
}
}
@@ -109,7 +107,7 @@ TMParse mktime_internal(const tm *tm_out) {
tm_out->tm_min * time_constants::SECONDS_PER_MIN +
tm_out->tm_hour * time_constants::SECONDS_PER_HOUR +
total_days * time_constants::SECONDS_PER_DAY;
- return {seconds, false};
+ return {seconds};
}
static int64_t computeRemainingYears(int64_t daysPerYears,
diff --git a/libc/src/time/time_utils.h b/libc/src/time/time_utils.h
index fb8058f0e72b6..1f1ffa07bcfcc 100644
--- a/libc/src/time/time_utils.h
+++ b/libc/src/time/time_utils.h
@@ -24,15 +24,9 @@
namespace LIBC_NAMESPACE_DECL {
namespace time_utils {
-
-// store the result of mktime_internal(const tm* )
-struct TMParse {
- time_t seconds;
- bool out_of_range_flag;
-};
// calculates the seconds from the epoch for tm_in. Does not update the struct,
// you must call update_from_seconds for that.
-TMParse mktime_internal(const tm *tm_out);
+cpp::optional<time_t> mktime_internal(const tm *tm_out);
// Update the "tm" structure's year, month, etc. members from seconds.
// "total_seconds" is the number of seconds since January 1st, 1970.
@@ -335,8 +329,11 @@ class TMReader final {
}
LIBC_INLINE time_t get_epoch() const {
- auto[seconds, _] = mktime_internal(timeptr);
- return seconds;
+ auto seconds = mktime_internal(timeptr);
+ if (seconds.has_value()) {
+ return seconds.value();
+ }
+ return time_utils::out_of_range();
}
// returns the timezone offset in microwave time:
More information about the libc-commits
mailing list