[libcxx] r273103 - Fix bugs in last_write_time implementation.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Sat Jun 18 19:04:50 PDT 2016
Author: ericwf
Date: Sat Jun 18 21:04:49 2016
New Revision: 273103
URL: http://llvm.org/viewvc/llvm-project?rev=273103&view=rev
Log:
Fix bugs in last_write_time implementation.
* Fix passing a negative number as either tv_usec or tv_nsec. When file_time_type
is negative and has a non-zero sub-second value we subtract 1 from tv_sec
and make the sub-second duration positive.
* Detect and report when 'file_time_type' cannot be represented by time_t. This
happens when using large/small file_time_type values with a 32 bit time_t.
There is more work to be done in the implementation. It should start to use
stat's st_mtim or st_mtimeval if it's provided as an extension. That way
we can provide a better resolution.
Modified:
libcxx/trunk/src/experimental/filesystem/operations.cpp
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/operations.cpp?rev=273103&r1=273102&r2=273103&view=diff
==============================================================================
--- libcxx/trunk/src/experimental/filesystem/operations.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/operations.cpp Sat Jun 18 21:04:49 2016
@@ -486,9 +486,45 @@ bool __fs_is_empty(const path& p, std::e
}
+namespace detail { namespace {
+
+template <class CType, class ChronoType>
+bool checked_set(CType* out, ChronoType time) {
+ using Lim = numeric_limits<CType>;
+ if (time > Lim::max() || time < Lim::min())
+ return false;
+ *out = static_cast<CType>(time);
+ return true;
+}
+
+constexpr long long min_seconds = file_time_type::duration::min().count()
+ / file_time_type::period::den;
+
+template <class SubSecDurT, class SubSecT>
+bool set_times_checked(time_t* sec_out, SubSecT* subsec_out, file_time_type tp) {
+ using namespace chrono;
+ auto dur = tp.time_since_epoch();
+ auto sec_dur = duration_cast<seconds>(dur);
+ auto subsec_dur = duration_cast<SubSecDurT>(dur - sec_dur);
+ // The tv_nsec and tv_usec fields must not be negative so adjust accordingly
+ if (subsec_dur.count() < 0) {
+ if (sec_dur.count() > min_seconds) {
+
+ sec_dur -= seconds(1);
+ subsec_dur += seconds(1);
+ } else {
+ subsec_dur = SubSecDurT::zero();
+ }
+ }
+ return checked_set(sec_out, sec_dur.count())
+ && checked_set(subsec_out, subsec_dur.count());
+}
+
+}} // end namespace detail
+
+
file_time_type __last_write_time(const path& p, std::error_code *ec)
{
- using Clock = file_time_type::clock;
std::error_code m_ec;
struct ::stat st;
detail::posix_stat(p, st, &m_ec);
@@ -497,10 +533,9 @@ file_time_type __last_write_time(const p
return file_time_type::min();
}
if (ec) ec->clear();
- return Clock::from_time_t(static_cast<std::time_t>(st.st_mtime));
+ return file_time_type::clock::from_time_t(st.st_mtime);
}
-
void __last_write_time(const path& p, file_time_type new_time,
std::error_code *ec)
{
@@ -513,34 +548,38 @@ void __last_write_time(const path& p, fi
// This implementation has a race condition between determining the
// last access time and attempting to set it to the same value using
// ::utimes
- using Clock = file_time_type::clock;
struct ::stat st;
file_status fst = detail::posix_stat(p, st, &m_ec);
if (m_ec && !status_known(fst)) {
set_or_throw(m_ec, ec, "last_write_time", p);
return;
}
- auto write_dur = new_time.time_since_epoch();
- auto write_sec = duration_cast<seconds>(write_dur);
- auto access_dur = Clock::from_time_t(st.st_atime).time_since_epoch();
- auto access_sec = duration_cast<seconds>(access_dur);
struct ::timeval tbuf[2];
- tbuf[0].tv_sec = access_sec.count();
- tbuf[0].tv_usec = duration_cast<microseconds>(access_dur - access_sec).count();
- tbuf[1].tv_sec = write_sec.count();
- tbuf[1].tv_usec = duration_cast<microseconds>(write_dur - write_sec).count();
+ tbuf[0].tv_sec = st.st_atime;
+ tbuf[0].tv_usec = 0;
+ const bool overflowed = !detail::set_times_checked<microseconds>(
+ &tbuf[1].tv_sec, &tbuf[1].tv_usec, new_time);
+
+ if (overflowed) {
+ set_or_throw(make_error_code(errc::invalid_argument), ec,
+ "last_write_time", p);
+ return;
+ }
if (::utimes(p.c_str(), tbuf) == -1) {
m_ec = detail::capture_errno();
}
#else
- auto dur_since_epoch = new_time.time_since_epoch();
- auto sec_since_epoch = duration_cast<seconds>(dur_since_epoch);
- auto ns_since_epoch = duration_cast<nanoseconds>(dur_since_epoch - sec_since_epoch);
struct ::timespec tbuf[2];
tbuf[0].tv_sec = 0;
tbuf[0].tv_nsec = UTIME_OMIT;
- tbuf[1].tv_sec = sec_since_epoch.count();
- tbuf[1].tv_nsec = ns_since_epoch.count();
+
+ const bool overflowed = !detail::set_times_checked<nanoseconds>(
+ &tbuf[1].tv_sec, &tbuf[1].tv_nsec, new_time);
+ if (overflowed) {
+ set_or_throw(make_error_code(errc::invalid_argument),
+ ec, "last_write_time", p);
+ return;
+ }
if (::utimensat(AT_FDCWD, p.c_str(), tbuf, 0) == -1) {
m_ec = detail::capture_errno();
}
Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?rev=273103&r1=273102&r2=273103&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp Sat Jun 18 21:04:49 2016
@@ -22,7 +22,6 @@
#include <type_traits>
#include <chrono>
#include <fstream>
-#include <iostream>
#include <cstdlib>
#include "test_macros.h"
@@ -30,6 +29,7 @@
#include "filesystem_test_helper.hpp"
#include <sys/stat.h>
+#include <iostream>
using namespace std::experimental::filesystem;
@@ -72,6 +72,13 @@ std::pair<std::time_t, std::time_t> GetS
return {st.st_atime, st.st_mtime};
}
+inline bool TimeIsRepresentableAsTimeT(file_time_type tp) {
+ using namespace std::chrono;
+ using Lim = std::numeric_limits<std::time_t>;
+ auto sec = duration_cast<seconds>(tp.time_since_epoch()).count();
+ return (sec >= Lim::min() && sec <= Lim::max());
+}
+
TEST_SUITE(exists_test_suite)
@@ -162,37 +169,58 @@ TEST_CASE(set_last_write_time_dynamic_en
using Sec = std::chrono::seconds;
using Hours = std::chrono::hours;
using Minutes = std::chrono::minutes;
-
+ using MicroSec = std::chrono::microseconds;
scoped_test_env env;
const path file = env.create_file("file", 42);
const path dir = env.create_dir("dir");
+ const auto now = Clock::now();
+ const file_time_type epoch_time = now - now.time_since_epoch();
- const file_time_type future_time = Clock::now() + Hours(3);
- const file_time_type past_time = Clock::now() - Minutes(3);
+ const file_time_type future_time = now + Hours(3) + Sec(42) + MicroSec(17);
+ const file_time_type past_time = now - Minutes(3) - Sec(42) - MicroSec(17);
+ const file_time_type before_epoch_time = epoch_time - Minutes(3) - Sec(42) - MicroSec(17);
+ // FreeBSD has a bug in their utimes implementation where the time is not update
+ // when the number of seconds is '-1'.
+#if defined(__FreeBSD__)
+ const file_time_type just_before_epoch_time = epoch_time - Sec(2) - MicroSec(17);
+#else
+ const file_time_type just_before_epoch_time = epoch_time - MicroSec(17);
+#endif
struct TestCase {
path p;
file_time_type new_time;
} cases[] = {
+ {file, epoch_time},
+ {dir, epoch_time},
{file, future_time},
{dir, future_time},
{file, past_time},
- {dir, past_time}
+ {dir, past_time},
+ {file, before_epoch_time},
+ {dir, before_epoch_time},
+ {file, just_before_epoch_time},
+ {dir, just_before_epoch_time}
};
for (const auto& TC : cases) {
const auto old_times = GetTimes(TC.p);
+ file_time_type old_time(Sec(old_times.second));
std::error_code ec = GetTestEC();
last_write_time(TC.p, TC.new_time, ec);
TEST_CHECK(!ec);
- const std::time_t new_time_t = Clock::to_time_t(TC.new_time);
file_time_type got_time = last_write_time(TC.p);
- std::time_t got_time_t = Clock::to_time_t(got_time);
- TEST_CHECK(got_time_t != old_times.second);
- TEST_CHECK(got_time_t == new_time_t);
+ TEST_CHECK(got_time != old_time);
+ if (TC.new_time < epoch_time) {
+ TEST_CHECK(got_time <= TC.new_time);
+ TEST_CHECK(got_time > TC.new_time - Sec(1));
+ } else {
+ TEST_CHECK(got_time <= TC.new_time + Sec(1));
+ TEST_CHECK(got_time >= TC.new_time - Sec(1));
+ }
TEST_CHECK(LastAccessTime(TC.p) == old_times.first);
}
}
@@ -229,41 +257,79 @@ TEST_CASE(last_write_time_symlink_test)
TEST_CHECK(GetSymlinkTimes(sym) == old_sym_times);
}
-TEST_CASE(test_write_min_max_time)
+
+TEST_CASE(test_write_min_time)
{
using Clock = file_time_type::clock;
using Sec = std::chrono::seconds;
- using Hours = std::chrono::hours;
+ using MicroSec = std::chrono::microseconds;
+ using Lim = std::numeric_limits<std::time_t>;
scoped_test_env env;
const path p = env.create_file("file", 42);
+ std::error_code ec = GetTestEC();
file_time_type last_time = last_write_time(p);
-
file_time_type new_time = file_time_type::min();
- std::error_code ec = GetTestEC();
+
last_write_time(p, new_time, ec);
file_time_type tt = last_write_time(p);
- if (ec) {
+
+ if (!TimeIsRepresentableAsTimeT(new_time)) {
+ TEST_CHECK(ec);
TEST_CHECK(ec != GetTestEC());
TEST_CHECK(tt == last_time);
} else {
- file_time_type max_allowed = new_time + Sec(1);
+ TEST_CHECK(!ec);
TEST_CHECK(tt >= new_time);
- TEST_CHECK(tt < max_allowed);
+ TEST_CHECK(tt < new_time + Sec(1));
}
- last_time = tt;
- new_time = file_time_type::max();
ec = GetTestEC();
- last_write_time(p, new_time, ec);
+ last_write_time(p, Clock::now());
+ last_time = last_write_time(p);
+
+ new_time = file_time_type::min() + MicroSec(1);
+ last_write_time(p, new_time, ec);
tt = last_write_time(p);
- if (ec) {
+
+ if (!TimeIsRepresentableAsTimeT(new_time)) {
+ TEST_CHECK(ec);
+ TEST_CHECK(ec != GetTestEC());
+ TEST_CHECK(tt == last_time);
+ } else {
+ TEST_CHECK(!ec);
+ TEST_CHECK(tt >= new_time);
+ TEST_CHECK(tt < new_time + Sec(1));
+ }
+}
+
+
+
+TEST_CASE(test_write_min_max_time)
+{
+ using Clock = file_time_type::clock;
+ using Sec = std::chrono::seconds;
+ using Hours = std::chrono::hours;
+ using Lim = std::numeric_limits<std::time_t>;
+ scoped_test_env env;
+ const path p = env.create_file("file", 42);
+
+ std::error_code ec = GetTestEC();
+ file_time_type last_time = last_write_time(p);
+ file_time_type new_time = file_time_type::max();
+
+ ec = GetTestEC();
+ last_write_time(p, new_time, ec);
+ file_time_type tt = last_write_time(p);
+
+ if (!TimeIsRepresentableAsTimeT(new_time)) {
+ TEST_CHECK(ec);
TEST_CHECK(ec != GetTestEC());
TEST_CHECK(tt == last_time);
} else {
- file_time_type min_allowed = new_time - Sec(1);
- TEST_CHECK(tt > min_allowed);
+ TEST_CHECK(!ec);
+ TEST_CHECK(tt > new_time - Sec(1));
TEST_CHECK(tt <= new_time);
}
}
More information about the cfe-commits
mailing list