[libcxx-commits] [PATCH] D91142: [10/N] [libcxx] Implement _FilesystemClock::now() and __last_write_time for windows

[Adrian McCarthy via Phabricator via libcxx-commits]

amccarth added a comment.

This looks good.  I'm just confused by one point and have a small suggested edit.



================
Comment at: libcxx/src/filesystem/operations.cpp:399
 #if defined(_LIBCPP_WIN32API)
 
 #define FILE_TIME_OFFSET_SECS (uint64_t(369 * 365 + 89) * (24 * 60 * 60))
----------------
Just a suggestion to give those magic values some context.


================
Comment at: libcxx/src/filesystem/operations.cpp:1227
+  if (!fs_time::convert_to_timespec(ts, new_time) ||
+      new_time == file_time_type::min())
+    return err.report(errc::value_too_large);
----------------
It's not clear to me why this checks `new_time == file_time_type::min()`.  It looks like convert_to_timespec returns false if the new time is not representable.  In what case would new_time be min?  And why would we then say the value is too large?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91142/new/

https://reviews.llvm.org/D91142