From 3293637e703bfc47e556db3b90e64fbceaede080 Mon Sep 17 00:00:00 2001 From: gm Date: Thu, 2 Jun 2016 09:46:27 +1200 Subject: [PATCH 1/5] Tighten up parsing. Detect missing quotes. Use istringstream for clarity. --- tz.cpp | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tz.cpp b/tz.cpp index 4435b7f..b120bc3 100644 --- a/tz.cpp +++ b/tz.cpp @@ -272,12 +272,6 @@ namespace // Put types in an anonymous name space. }; } // anonymous namespace -template < typename T, size_t N > -static inline size_t countof(T(&arr)[N]) -{ - return std::extent< T[N] >::value; -} - // This function returns an exhaustive list of time zone information // from the Windows registry. // The routine tries to load as many time zone entries as possible despite errors. @@ -397,7 +391,7 @@ load_timezone_mappings_from_csv_file(const std::string& input_path) throw std::runtime_error(msg); } - std::stringstream sis; + std::istringstream sis; auto error = [&](const char* info) { std::string msg = "Error reading zone mapping file \""; @@ -408,7 +402,14 @@ load_timezone_mappings_from_csv_file(const std::string& input_path) msg += info; throw std::runtime_error(msg); }; - + auto read_field_quote = [&]() + { + char field_delim; + sis.read(&field_delim, 1); + auto read_count = sis.gcount(); + if (sis.gcount() != 1 || field_delim != '"') + error("field '\"' expected."); + }; auto read_field_delim = [&]() { char field_delim; @@ -460,19 +461,17 @@ load_timezone_mappings_from_csv_file(const std::string& input_path) sis.str(linebuf); char ch; - sis.read(&ch, 1); + read_field_quote(); std::getline(sis, zm.other, '\"'); read_field_delim(); - sis.read(&ch, 1); + read_field_quote(); std::getline(sis, zm.territory, '\"'); read_field_delim(); - sis.read(&ch, 1); + read_field_quote(); std::getline(sis, zm.type, '\"'); - - sis.read(&ch, 1); - if (!sis.eof()) // Excess characters? We should have processed all in the line buffer. + if ((size_t)sis.tellg() != linebuf.length()) // Excess characters? We should have processed all in the line buffer. error("Formatting error."); ++line; mappings.push_back(std::move(zm)); From 09cf3bba8da6b1137343b0825ad7e09adb6cdea0 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 1 Jun 2016 23:25:07 -0400 Subject: [PATCH 2/5] Create %Ez & %Oz to put ':' in offset for format and parse. --- tz.h | 134 +++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 94 insertions(+), 40 deletions(-) diff --git a/tz.h b/tz.h index e9619a5..9b9e4b7 100644 --- a/tz.h +++ b/tz.h @@ -48,6 +48,8 @@ On Windows, the names are never "Standard" so mapping is always required. Technically any OS may use the mapping process but currently only Windows does use it. */ +#include + #ifdef _WIN32 #ifndef TIMEZONE_MAPPING #define TIMEZONE_MAPPING 1 @@ -1091,31 +1093,44 @@ format(const std::locale& loc, std::basic_string format, // Handle these specially // %S append fractional seconds if tp has precision finer than seconds // %T append fractional seconds if tp has precision finer than seconds - // %z replace with offset from zone + // %z replace with offset from zone on +/-hhmm format + // %Ez, %Oz replace with offset from zone on +/-hh:mm format // %Z replace with abbreviation from zone using namespace std; using namespace std::chrono; + auto command = false; + auto modified = false; for (std::size_t i = 0; i < format.size(); ++i) { - if (format[i] == '%' && i < format.size()-1) + switch (format[i]) { - switch (format[i+1]) + case '%': + command = true; + modified = false; + break; + case 'O': + case 'E': + modified = true; + break; + case 'S': + case 'T': + if (command && !modified && ratio_less>::value) + { + basic_ostringstream os; + os.imbue(loc); + os << make_time(tp - floor(tp)); + auto s = os.str(); + s.erase(0, 8); + format.insert(i+1, s); + i += s.size() - 1; + } + command = false; + modified = false; + break; + case 'z': + if (command) { - case 'S': - case 'T': - if (ratio_less>::value) - { - basic_ostringstream os; - os.imbue(loc); - os << make_time(tp - floor(tp)); - auto s = os.str(); - s.erase(0, 8); - format.insert(i+2, s); - i += 2 + s.size() - 1; - } - break; - case 'z': if (zone == nullptr) throw std::runtime_error("Can not format local_time with %z"); else @@ -1127,12 +1142,18 @@ format(const std::locale& loc, std::basic_string format, os << '+'; os << make_time(offset); auto s = os.str(); - s.erase(s.find(':'), 1); - format.replace(i, 2, s); + if (!modified) + s.erase(s.find(':'), 1); + format.replace(i - 1 - modified, 2 + modified, s); i += s.size() - 1; } - break; - case 'Z': + } + command = false; + modified = false; + break; + case 'Z': + if (command && !modified) + { if (zone == nullptr) throw std::runtime_error("Can not format local_time with %z"); else @@ -1142,8 +1163,14 @@ format(const std::locale& loc, std::basic_string format, (info.abbrev.begin(), info.abbrev.end())); i += info.abbrev.size() - 1; } - break; } + command = false; + modified = false; + break; + default: + command = false; + modified = false; + break; } } auto& f = use_facet>(loc); @@ -1296,21 +1323,30 @@ parse(std::basic_istream& is, auto b = format.data(); auto i = b; auto e = b + format.size(); + auto command = false; + auto modified = false; for (; i < e; ++i) { - if (*i == '%' && i < e-1) + switch (*i) { - switch (i[1]) + case '%': + command = true; + modified = false; + break; + case 'O': + case 'E': + modified = true; + break; + case 'T': + case 'S': + if (command && !modified) { - case 'T': - case 'S': - f.get(is, 0, is, err, &tm, b, i); - ++i; + f.get(is, 0, is, err, &tm, b, i-1); b = i+1; if (*i == 'T') { - const CharT hm[] = {'%', 'H', ':', '%', 'M', ':', 0}; - f.get(is, 0, is, err, &tm, hm, hm); + const CharT hm[] = {'%', 'H', ':', '%', 'M', ':'}; + f.get(is, 0, is, err, &tm, hm, hm+6); } if (ratio_less>::value) { @@ -1326,10 +1362,14 @@ parse(std::basic_istream& is, const CharT hm[] = {'%', 'S'}; f.get(is, 0, is, err, &tm, hm, hm+2); } - break; - case 'z': - f.get(is, 0, is, err, &tm, b, i); - ++i; + } + command = false; + modified = false; + break; + case 'z': + if (command) + { + f.get(is, 0, is, err, &tm, b, i-1-modified); b = i+1; if ((err & ios_base::failbit) == 0) { @@ -1338,12 +1378,16 @@ parse(std::basic_istream& is, if (!is.fail() && (sign == '+' || sign == '-')) { char h1, h0, m1, m0; + char colon = '\0'; h1 = static_cast(is.get()); h0 = static_cast(is.get()); + if (modified) + colon = static_cast(is.get()); m1 = static_cast(is.get()); m0 = static_cast(is.get()); if (!is.fail() && std::isdigit(h1) && std::isdigit(h0) - && std::isdigit(m1) && std::isdigit(m0)) + && std::isdigit(m1) && std::isdigit(m0) + && (!modified || colon == ':')) { temp_offset = 10*hours{h1 - '0'} + hours{h0 - '0'} + 10*minutes{m1 - '0'} + minutes{m0 - '0'}; @@ -1356,10 +1400,14 @@ parse(std::basic_istream& is, else err |= ios_base::failbit; } - break; - case 'Z': - f.get(is, 0, is, err, &tm, b, i); - ++i; + } + command = false; + modified = false; + break; + case 'Z': + if (command && !modified) + { + f.get(is, 0, is, err, &tm, b, i-1); b = i+1; if ((err & ios_base::failbit) == 0) { @@ -1367,8 +1415,14 @@ parse(std::basic_istream& is, if (is.fail()) err |= ios_base::failbit; } - break; } + command = false; + modified = false; + break; + default: + command = false; + modified = false; + break; } } if ((err & ios_base::failbit) == 0) From 10f2ae9e6c283bc999c976b88ce03493b3762424 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Fri, 3 Jun 2016 11:28:30 -0400 Subject: [PATCH 3/5] Update README.md --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 22bac82..19f8d4d 100644 --- a/README.md +++ b/README.md @@ -11,3 +11,7 @@ This is actually three separate C++11/C++14 libraries: 3. `"iso_week.h"` is a header-only library built on top of the `"date.h"` library which implements the ISO week date calendar. See http://howardhinnant.github.io/date/iso_week.html for more details. There has been a recent change in the library design. If you are trying to migrate from the previous design, rename `day_point` to `sys_days` everywhere, and that ought to bring the number of errors down to a small roar. + +`"date.h"` and `"tz.h"` are now proposed for standardization here: + + http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0355r0.html From bf505cc66ac2724a050991e21f458354ccd57fb0 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Fri, 3 Jun 2016 11:29:17 -0400 Subject: [PATCH 4/5] Update README.md --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 19f8d4d..3e385f6 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,4 @@ This is actually three separate C++11/C++14 libraries: There has been a recent change in the library design. If you are trying to migrate from the previous design, rename `day_point` to `sys_days` everywhere, and that ought to bring the number of errors down to a small roar. -`"date.h"` and `"tz.h"` are now proposed for standardization here: - - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0355r0.html +`"date.h"` and `"tz.h"` are now proposed for standardization here: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0355r0.html From 7e66cb2e833c34cde61745f236d8b7ea385bdad8 Mon Sep 17 00:00:00 2001 From: gm Date: Fri, 3 Jun 2016 21:10:53 +1200 Subject: [PATCH 5/5] Improve cross platform support. Make validate work for C++11 because the library supports it. Fix an unused variable mistake i made earlier. Make constructor public, seems it should be. Possible compiler bug? --- test/tz_test/validate.cpp | 12 ++++++------ tz.cpp | 3 +-- tz.h | 9 +++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/tz_test/validate.cpp b/test/tz_test/validate.cpp index 5a7ee08..44d9036 100644 --- a/test/tz_test/validate.cpp +++ b/test/tz_test/validate.cpp @@ -101,14 +101,14 @@ tzmain() { std::cout << name << '\n'; auto z = locate_zone(name); - auto begin = sys_days(jan/1/year::min()) + 0s; - auto end = sys_days(jan/1/2035) + 0s; + auto begin = sys_days(jan/1/year::min()) + seconds{0}; + auto end = sys_days(jan/1/2035) + seconds{0}; auto info = z->get_info(begin); std::cout << "Initially: "; - if (info.offset >= 0s) + if (info.offset >= seconds{0}) std::cout << '+'; std::cout << make_time(info.offset); - if (info.save == 0min) + if (info.save == minutes{0}) std::cout << " standard "; else std::cout << " daylight "; @@ -128,10 +128,10 @@ tzmain() auto ymd = year_month_day(dp); auto time = make_time(begin - dp); std::cout << ymd << ' ' << time << "Z "; - if (info.offset >= 0s) + if (info.offset >= seconds{0}) std::cout << '+'; std::cout << make_time(info.offset); - if (info.save == 0min) + if (info.save == minutes{0}) std::cout << " standard "; else std::cout << " daylight "; diff --git a/tz.cpp b/tz.cpp index b120bc3..21adb24 100644 --- a/tz.cpp +++ b/tz.cpp @@ -126,7 +126,7 @@ CONSTDATA auto max_day = date::dec/31; namespace detail { -class undocumented {explicit undocumented() = default;}; +struct undocumented {explicit undocumented() = default;}; } #ifndef _MSC_VER @@ -460,7 +460,6 @@ load_timezone_mappings_from_csv_file(const std::string& input_path) sis.clear(); sis.str(linebuf); - char ch; read_field_quote(); std::getline(sis, zm.other, '\"'); read_field_delim(); diff --git a/tz.h b/tz.h index 9b9e4b7..254a68d 100644 --- a/tz.h +++ b/tz.h @@ -95,6 +95,7 @@ static_assert(HAS_REMOTE_API == 0 ? AUTO_DOWNLOAD == 0 : true, # include # include #endif +#include namespace date { @@ -103,7 +104,7 @@ enum class choose {earliest, latest}; namespace detail { - class undocumented; + struct undocumented; } class nonexistent_local_time @@ -1431,10 +1432,10 @@ parse(std::basic_istream& is, f.get(is, 0, is, err, &tm, b, e); if ((err & ios_base::failbit) == 0) { -#ifndef _MSC_VER - auto tt = timegm(&tm); -#else +#if _WIN32 auto tt = _mkgmtime(&tm); +#else + auto tt = timegm(&tm); #endif tp = floor(system_clock::from_time_t(tt) + subseconds); abbrev = std::move(temp_abbrev);