kubkon opened PR #1255 from filetime-in-yanix
to master
:
I've noticed that we could replace every occurrence of
crate::Result
infiletime
mods withio::Result
, so I thought why not move it
toyanix
and get rid off a lot of unnecessary code duplication
withinwasi-common
. Now, ideally I'd have ourfiletime
modifications
backported to @alexcrichton's [filetime
] crate, but one step at a time
(apologies @alexcrichton, I was meant to backport this ages ago, just didn't
find the time yet... :-().Anyway, this commit does just that; i.e., moves the
filetime
modules
intoyanix
which seems a better fit for this type of code.[
filetime
]: https://github.com/alexcrichton/filetimeThere is one caveat here. On Emscripten, converting between
filetime::Filetime
andlibc::timespec
appears to be lossy, at least as far as the
types are concerned. Now,filetime::Filetime
's seconds field is
i64
while nanoseconds field isu32
, while Emscripten's
libc::timespec
requires both to bei32
width. This might actually
not be a problem since I don't think it's possible to fillfiletime::Filetime
struct with values of width wider thani32
since Emscripten is 32bit
but just to be on the safe side, we do aTryInto
conversion, log
the error (if any), and returnlibc::EOVERFLOW
.Lemme know what you guys reckon!
kubkon requested alexcrichton, and sunfishcode for a review on PR #1255.
kubkon requested alexcrichton, and sunfishcode for a review on PR #1255.
kubkon updated PR #1255 from filetime-in-yanix
to master
:
I've noticed that we could replace every occurrence of
crate::Result
infiletime
mods withio::Result
, so I thought why not move it
toyanix
and get rid off a lot of unnecessary code duplication
withinwasi-common
. Now, ideally I'd have ourfiletime
modifications
backported to @alexcrichton's [filetime
] crate, but one step at a time
(apologies @alexcrichton, I was meant to backport this ages ago, just didn't
find the time yet... :-().Anyway, this commit does just that; i.e., moves the
filetime
modules
intoyanix
which seems a better fit for this type of code.[
filetime
]: https://github.com/alexcrichton/filetimeThere is one caveat here. On Emscripten, converting between
filetime::Filetime
andlibc::timespec
appears to be lossy, at least as far as the
types are concerned. Now,filetime::Filetime
's seconds field is
i64
while nanoseconds field isu32
, while Emscripten's
libc::timespec
requires both to bei32
width. This might actually
not be a problem since I don't think it's possible to fillfiletime::Filetime
struct with values of width wider thani32
since Emscripten is 32bit
but just to be on the safe side, we do aTryInto
conversion, log
the error (if any), and returnlibc::EOVERFLOW
.Lemme know what you guys reckon!
sunfishcode submitted PR Review.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
Slightly more verbose, but avoids the need for a two-line comment:
i64::from(ft.nanoseconds())
:-).
kubkon submitted PR Review.
kubkon created PR Review Comment:
Excellent, thanks!
kubkon updated PR #1255 from filetime-in-yanix
to master
:
I've noticed that we could replace every occurrence of
crate::Result
infiletime
mods withio::Result
, so I thought why not move it
toyanix
and get rid off a lot of unnecessary code duplication
withinwasi-common
. Now, ideally I'd have ourfiletime
modifications
backported to @alexcrichton's [filetime
] crate, but one step at a time
(apologies @alexcrichton, I was meant to backport this ages ago, just didn't
find the time yet... :-().Anyway, this commit does just that; i.e., moves the
filetime
modules
intoyanix
which seems a better fit for this type of code.[
filetime
]: https://github.com/alexcrichton/filetimeThere is one caveat here. On Emscripten, converting between
filetime::Filetime
andlibc::timespec
appears to be lossy, at least as far as the
types are concerned. Now,filetime::Filetime
's seconds field is
i64
while nanoseconds field isu32
, while Emscripten's
libc::timespec
requires both to bei32
width. This might actually
not be a problem since I don't think it's possible to fillfiletime::Filetime
struct with values of width wider thani32
since Emscripten is 32bit
but just to be on the safe side, we do aTryInto
conversion, log
the error (if any), and returnlibc::EOVERFLOW
.Lemme know what you guys reckon!
kubkon updated PR #1255 from filetime-in-yanix
to master
:
I've noticed that we could replace every occurrence of
crate::Result
infiletime
mods withio::Result
, so I thought why not move it
toyanix
and get rid off a lot of unnecessary code duplication
withinwasi-common
. Now, ideally I'd have ourfiletime
modifications
backported to @alexcrichton's [filetime
] crate, but one step at a time
(apologies @alexcrichton, I was meant to backport this ages ago, just didn't
find the time yet... :-().Anyway, this commit does just that; i.e., moves the
filetime
modules
intoyanix
which seems a better fit for this type of code.[
filetime
]: https://github.com/alexcrichton/filetimeThere is one caveat here. On Emscripten, converting between
filetime::Filetime
andlibc::timespec
appears to be lossy, at least as far as the
types are concerned. Now,filetime::Filetime
's seconds field is
i64
while nanoseconds field isu32
, while Emscripten's
libc::timespec
requires both to bei32
width. This might actually
not be a problem since I don't think it's possible to fillfiletime::Filetime
struct with values of width wider thani32
since Emscripten is 32bit
but just to be on the safe side, we do aTryInto
conversion, log
the error (if any), and returnlibc::EOVERFLOW
.Lemme know what you guys reckon!
kubkon merged PR #1255.
Last updated: Dec 23 2024 at 12:05 UTC