> On Nov. 14, 2017, 3:23 p.m., Andrei Budnik wrote: > > 3rdparty/stout/include/stout/os/linux.hpp > > Line 66 (original), 66 (patched) > > <https://reviews.apache.org/r/63677/diff/1/?file=1884914#file1884914line67> > > > > `ErrnoError` is not async-signal-safe as it constructs empty > > `std::string` in c'tor. > > > > I would suggest to change the return type of `allocate()` to `int` and > > get rid of `create` static function. > > James Peach wrote: > Yup, this isn't intended to be async-safe. If you need that, you are > supposed to call `allocate` manually. I'm planning make a change to propagate > this `Error` in MESOS-8155. > > Why do you suggest `int` as the return type? To match the syscall > convention, or to return an `errno` directly?
Ah, got it. Yeah, I meant matching syscall convention. If you decide to leave `create`, it would be nice to add a comment like: ```c++ // NOTE: this function is not async-signal safe. ``` - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63677/#review190945 ----------------------------------------------------------- On Nov. 14, 2017, 7:11 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63677/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2017, 7:11 p.m.) > > > Review request for mesos, Benjamin Bannier and Jie Yu. > > > Bugs: MESOS-8159 > https://issues.apache.org/jira/browse/MESOS-8159 > > > Repository: mesos > > > Description > ------- > > If `os:Stack` is allocated with `mmap`, this automatically > guarantees the correct alignment (since `mmap` is page-aligned > by definition). > > Although `mmap` is not required to be async signal safe > (i.e. it might not be safe to call from a signal handler), in > practice it should be safe to call between `fork` and `exec` > since it is a raw system call that does not require any user > space locks to be held. > > > Diffs > ----- > > 3rdparty/stout/include/stout/os/linux.hpp > 68b4090aa5e6f23609c487128d91301755ae0e46 > > > Diff: https://reviews.apache.org/r/63677/diff/2/ > > > Testing > ------- > > sudo make check (Fedora 26) > > > Thanks, > > James Peach > >
