Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement glibc ext for sec, min, and hour #3271

Merged
merged 6 commits into from
Feb 8, 2023
+151 −40
Merged

Implement glibc ext for sec, min, and hour #3271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dda4620
Implement glibc ext for sec, min, and hour
ShawnZhong Jan 13, 2023
f6efeba
Add comments
ShawnZhong Jan 13, 2023
43c2eae
Support floating point sec
ShawnZhong Jan 13, 2023
6a0f6ff
Fix build
ShawnZhong Jan 13, 2023
5dc570c
Fix bug found by fuzzing
ShawnZhong Jan 13, 2023
64951f1
Fix buffer overflow
ShawnZhong Jan 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 98 additions & 40 deletions include/fmt/chrono.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,30 @@ enum class numeric_system {
alternative
};

// Glibc extensions for formatting numeric values.
enum class pad_type {
unspecified,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need unspecified? It looks like it's always handled as zero and can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the current codebase, yes.

// Do not pad a numeric result string.
none,
// Pad a numeric result string with zeros even if the conversion specifier
// character uses space-padding by default.
Comment on lines +672 to +673
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which conversion specifiers use space padding by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, the command date "+%10a" prints Tue on Ubuntu 22.04.1.
If we were to implement the glibc extension for the optional width, we might need to consider this padding. Otherwise, we don't have space padding in the current codebase.

zero,
// Pad a numeric result string with spaces.
space,
};

template <typename OutputIt>
auto write_padding(OutputIt out, pad_type pad, int width) -> OutputIt {
if (pad == pad_type::none) return out;
return std::fill_n(out, width, pad == pad_type::space ? ' ' : '0');
}

template <typename OutputIt>
auto write_padding(OutputIt out, pad_type pad) -> OutputIt {
if (pad != pad_type::none) *out++ = pad == pad_type::space ? ' ' : '0';
return out;
}

// Parses a put_time-like format string and invokes handler actions.
template <typename Char, typename Handler>
FMT_CONSTEXPR const Char* parse_chrono_format(const Char* begin,
Expand All @@ -672,6 +696,7 @@ FMT_CONSTEXPR const Char* parse_chrono_format(const Char* begin,
if (begin == end || *begin == '}') return begin;
if (*begin != '%') FMT_THROW(format_error("invalid format"));
auto ptr = begin;
pad_type pad = pad_type::unspecified;
while (ptr != end) {
auto c = *ptr;
if (c == '}') break;
Expand All @@ -682,6 +707,22 @@ FMT_CONSTEXPR const Char* parse_chrono_format(const Char* begin,
if (begin != ptr) handler.on_text(begin, ptr);
++ptr; // consume '%'
if (ptr == end) FMT_THROW(format_error("invalid format"));
c = *ptr;
switch (c) {
case '_':
pad = pad_type::space;
++ptr;
break;
case '-':
pad = pad_type::none;
++ptr;
break;
case '0':
pad = pad_type::zero;
++ptr;
break;
}
if (ptr == end) FMT_THROW(format_error("invalid format"));
c = *ptr++;
switch (c) {
case '%':
Expand Down Expand Up @@ -758,16 +799,16 @@ FMT_CONSTEXPR const Char* parse_chrono_format(const Char* begin,
break;
// Hour, minute, second:
case 'H':
handler.on_24_hour(numeric_system::standard);
handler.on_24_hour(numeric_system::standard, pad);
break;
case 'I':
handler.on_12_hour(numeric_system::standard);
handler.on_12_hour(numeric_system::standard, pad);
break;
case 'M':
handler.on_minute(numeric_system::standard);
handler.on_minute(numeric_system::standard, pad);
break;
case 'S':
handler.on_second(numeric_system::standard);
handler.on_second(numeric_system::standard, pad);
break;
// Other:
case 'c':
Expand Down Expand Up @@ -872,16 +913,16 @@ FMT_CONSTEXPR const Char* parse_chrono_format(const Char* begin,
handler.on_dec1_weekday(numeric_system::alternative);
break;
case 'H':
handler.on_24_hour(numeric_system::alternative);
handler.on_24_hour(numeric_system::alternative, pad);
break;
case 'I':
handler.on_12_hour(numeric_system::alternative);
handler.on_12_hour(numeric_system::alternative, pad);
break;
case 'M':
handler.on_minute(numeric_system::alternative);
handler.on_minute(numeric_system::alternative, pad);
break;
case 'S':
handler.on_second(numeric_system::alternative);
handler.on_second(numeric_system::alternative, pad);
break;
case 'z':
handler.on_utc_offset(numeric_system::alternative);
Expand Down Expand Up @@ -965,10 +1006,10 @@ struct tm_format_checker : null_chrono_spec_handler<tm_format_checker> {
FMT_CONSTEXPR void on_day_of_year() {}
FMT_CONSTEXPR void on_day_of_month(numeric_system) {}
FMT_CONSTEXPR void on_day_of_month_space(numeric_system) {}
FMT_CONSTEXPR void on_24_hour(numeric_system) {}
FMT_CONSTEXPR void on_12_hour(numeric_system) {}
FMT_CONSTEXPR void on_minute(numeric_system) {}
FMT_CONSTEXPR void on_second(numeric_system) {}
FMT_CONSTEXPR void on_24_hour(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_12_hour(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_minute(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_second(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_datetime(numeric_system) {}
FMT_CONSTEXPR void on_loc_date(numeric_system) {}
FMT_CONSTEXPR void on_loc_time(numeric_system) {}
Expand Down Expand Up @@ -1238,6 +1279,17 @@ class tm_writer {
*out_++ = *d++;
*out_++ = *d;
}
void write2(int value, pad_type pad) {
unsigned int v = to_unsigned(value) % 100;
if (v >= 10) {
const char* d = digits2(v);
*out_++ = *d++;
*out_++ = *d;
} else {
out_ = detail::write_padding(out_, pad);
*out_++ = static_cast<char>('0' + v);
}
}

void write_year_extended(long long year) {
// At least 4 characters.
Expand Down Expand Up @@ -1514,23 +1566,25 @@ class tm_writer {
}
}

void on_24_hour(numeric_system ns) {
if (is_classic_ || ns == numeric_system::standard) return write2(tm_hour());
void on_24_hour(numeric_system ns, pad_type pad) {
if (is_classic_ || ns == numeric_system::standard)
return write2(tm_hour(), pad);
format_localized('H', 'O');
}
void on_12_hour(numeric_system ns) {
void on_12_hour(numeric_system ns, pad_type pad) {
if (is_classic_ || ns == numeric_system::standard)
return write2(tm_hour12());
return write2(tm_hour12(), pad);
format_localized('I', 'O');
}
void on_minute(numeric_system ns) {
if (is_classic_ || ns == numeric_system::standard) return write2(tm_min());
void on_minute(numeric_system ns, pad_type pad) {
if (is_classic_ || ns == numeric_system::standard)
return write2(tm_min(), pad);
format_localized('M', 'O');
}

void on_second(numeric_system ns) {
void on_second(numeric_system ns, pad_type pad) {
if (is_classic_ || ns == numeric_system::standard) {
write2(tm_sec());
write2(tm_sec(), pad);
if (subsecs_) {
if (std::is_floating_point<typename Duration::rep>::value) {
auto buf = memory_buffer();
Expand Down Expand Up @@ -1594,10 +1648,10 @@ struct chrono_format_checker : null_chrono_spec_handler<chrono_format_checker> {

template <typename Char>
FMT_CONSTEXPR void on_text(const Char*, const Char*) {}
FMT_CONSTEXPR void on_24_hour(numeric_system) {}
FMT_CONSTEXPR void on_12_hour(numeric_system) {}
FMT_CONSTEXPR void on_minute(numeric_system) {}
FMT_CONSTEXPR void on_second(numeric_system) {}
FMT_CONSTEXPR void on_24_hour(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_12_hour(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_minute(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_second(numeric_system, pad_type) {}
FMT_CONSTEXPR void on_12_hour_time() {}
FMT_CONSTEXPR void on_24_hour_time() {}
FMT_CONSTEXPR void on_iso_time() {}
Expand Down Expand Up @@ -1819,13 +1873,15 @@ struct chrono_formatter {
}
}

void write(Rep value, int width) {
void write(Rep value, int width, pad_type pad = pad_type::unspecified) {
write_sign();
if (isnan(value)) return write_nan();
uint32_or_64_or_128_t<int> n =
to_unsigned(to_nonnegative_int(value, max_value<int>()));
int num_digits = detail::count_digits(n);
if (width > num_digits) out = std::fill_n(out, width - num_digits, '0');
if (width > num_digits) {
out = detail::write_padding(out, pad, width - num_digits);
}
out = format_decimal<char_type>(out, n, num_digits).end;
}

Expand Down Expand Up @@ -1874,34 +1930,34 @@ struct chrono_formatter {
void on_day_of_month(numeric_system) {}
void on_day_of_month_space(numeric_system) {}

void on_24_hour(numeric_system ns) {
void on_24_hour(numeric_system ns, pad_type pad) {
if (handle_nan_inf()) return;

if (ns == numeric_system::standard) return write(hour(), 2);
if (ns == numeric_system::standard) return write(hour(), 2, pad);
auto time = tm();
time.tm_hour = to_nonnegative_int(hour(), 24);
format_tm(time, &tm_writer_type::on_24_hour, ns);
format_tm(time, &tm_writer_type::on_24_hour, ns, pad);
}

void on_12_hour(numeric_system ns) {
void on_12_hour(numeric_system ns, pad_type pad) {
if (handle_nan_inf()) return;

if (ns == numeric_system::standard) return write(hour12(), 2);
if (ns == numeric_system::standard) return write(hour12(), 2, pad);
auto time = tm();
time.tm_hour = to_nonnegative_int(hour12(), 12);
format_tm(time, &tm_writer_type::on_12_hour, ns);
format_tm(time, &tm_writer_type::on_12_hour, ns, pad);
}

void on_minute(numeric_system ns) {
void on_minute(numeric_system ns, pad_type pad) {
if (handle_nan_inf()) return;

if (ns == numeric_system::standard) return write(minute(), 2);
if (ns == numeric_system::standard) return write(minute(), 2, pad);
auto time = tm();
time.tm_min = to_nonnegative_int(minute(), 60);
format_tm(time, &tm_writer_type::on_minute, ns);
format_tm(time, &tm_writer_type::on_minute, ns, pad);
}

void on_second(numeric_system ns) {
void on_second(numeric_system ns, pad_type pad) {
if (handle_nan_inf()) return;

if (ns == numeric_system::standard) {
Expand All @@ -1910,18 +1966,20 @@ struct chrono_formatter {
write_floating_seconds(buf, std::chrono::duration<rep, Period>(val),
precision);
if (negative) *out++ = '-';
if (buf.size() < 2 || buf[1] == '.') *out++ = '0';
if (buf.size() < 2 || buf[1] == '.') {
out = detail::write_padding(out, pad);
}
out = std::copy(buf.begin(), buf.end(), out);
} else {
write(second(), 2);
write(second(), 2, pad);
write_fractional_seconds<char_type>(
out, std::chrono::duration<rep, Period>(val), precision);
}
return;
}
auto time = tm();
time.tm_sec = to_nonnegative_int(second(), 60);
format_tm(time, &tm_writer_type::on_second, ns);
format_tm(time, &tm_writer_type::on_second, ns, pad);
}

void on_12_hour_time() {
Expand All @@ -1945,7 +2003,7 @@ struct chrono_formatter {
on_24_hour_time();
*out++ = ':';
if (handle_nan_inf()) return;
on_second(numeric_system::standard);
on_second(numeric_system::standard, pad_type::unspecified);
}

void on_am_pm() {
Expand Down
53 changes: 53 additions & 0 deletions test/chrono-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,3 +919,56 @@ TEST(chrono_test, timestamps_sub_seconds) {
EXPECT_EQ("00.250", fmt::format("{:%S}", epoch + d));
}
}

TEST(chrono_test, glibc_extensions) {
EXPECT_THROW_MSG((void)fmt::format(runtime("{:%0}"), std::chrono::seconds()),
fmt::format_error, "invalid format");
EXPECT_THROW_MSG((void)fmt::format(runtime("{:%_}"), std::chrono::seconds()),
fmt::format_error, "invalid format");
EXPECT_THROW_MSG((void)fmt::format(runtime("{:%-}"), std::chrono::seconds()),
fmt::format_error, "invalid format");

{
const auto d = std::chrono::hours(1) + std::chrono::minutes(2) +
std::chrono::seconds(3);

EXPECT_EQ(fmt::format("{:%I,%H,%M,%S}", d), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%0I,%0H,%0M,%0S}", d), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%_I,%_H,%_M,%_S}", d), " 1, 1, 2, 3");
EXPECT_EQ(fmt::format("{:%-I,%-H,%-M,%-S}", d), "1,1,2,3");

EXPECT_EQ(fmt::format("{:%OI,%OH,%OM,%OS}", d), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%0OI,%0OH,%0OM,%0OS}", d), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%_OI,%_OH,%_OM,%_OS}", d), " 1, 1, 2, 3");
EXPECT_EQ(fmt::format("{:%-OI,%-OH,%-OM,%-OS}", d), "1,1,2,3");
}

{
const auto tm = make_tm(1970, 1, 1, 1, 2, 3);
EXPECT_EQ(fmt::format("{:%I,%H,%M,%S}", tm), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%0I,%0H,%0M,%0S}", tm), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%_I,%_H,%_M,%_S}", tm), " 1, 1, 2, 3");
EXPECT_EQ(fmt::format("{:%-I,%-H,%-M,%-S}", tm), "1,1,2,3");

EXPECT_EQ(fmt::format("{:%OI,%OH,%OM,%OS}", tm), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%0OI,%0OH,%0OM,%0OS}", tm), "01,01,02,03");
EXPECT_EQ(fmt::format("{:%_OI,%_OH,%_OM,%_OS}", tm), " 1, 1, 2, 3");
EXPECT_EQ(fmt::format("{:%-OI,%-OH,%-OM,%-OS}", tm), "1,1,2,3");
}

{
const auto d = std::chrono::seconds(3) + std::chrono::milliseconds(140);
EXPECT_EQ(fmt::format("{:%S}", d), "03.140");
EXPECT_EQ(fmt::format("{:%0S}", d), "03.140");
EXPECT_EQ(fmt::format("{:%_S}", d), " 3.140");
EXPECT_EQ(fmt::format("{:%-S}", d), "3.140");
}

{
const auto d = std::chrono::duration<double>(3.14);
EXPECT_EQ(fmt::format("{:%S}", d), "03.140000");
EXPECT_EQ(fmt::format("{:%0S}", d), "03.140000");
EXPECT_EQ(fmt::format("{:%_S}", d), " 3.140000");
EXPECT_EQ(fmt::format("{:%-S}", d), "3.140000");
}
}