> On Feb. 25, 2019, 10:05 p.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/os/posix/mktemp.hpp > > Lines 39-57 (original), 39-57 (patched) > > <https://reviews.apache.org/r/70053/diff/1/?file=2126763#file2126763line39> > > > > From what I understand, C++11 already guarantees for us that > > `_path[_path.size()] == '\0'` and so we shouldn't need this +1 and '\0' > > logic? > > > > ``` > > std::string _path(path); > > > > int_fd fd = ::mkstemp(&_path[0]); > > > > ... > > > > return _path; > > ``` > > Benjamin Bannier wrote: > I read that, too. > > Note that here we do not evaluate `_path[_path.size()]` directly, but > instead pass `&_path[0]` to a function expecting a null-terminated string. > AFAICT no C++ standard makes promises about `string` actually > null-terminating strings internally (e.g., `_path.data()` which would be > equivalent to `&_path[0]` isn't per se uaranteed to point to a > null-terminated string in even the most recent standard), and the only > reference to automatically null-terminating strings I could find were related > to explicitly accessing the `size()`'th element, and `string::c_str`, neither > of which return the internal buffer we are using here. > > I believe what I do here is needed.
I was chatting with mpark, it seems the following in the standard enforces it: > data() / c_str(): > > A pointer p such that p + i == &operator[](i) for each i in [0, size()] in combination with: > const_reference operator[](size_type pos) const > > Returns: *(begin() + pos) if pos < size(). Otherwise, returns a reference to > an object of type charT with value charT(), where modifying the object to any value other than charT() leads to undefined behavior i.e. if you pass s.data(), it needs to be null terminated. Not sure where you saw the information about .data()? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70053/#review213187 ----------------------------------------------------------- On Feb. 25, 2019, 9:20 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70053/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2019, 9:20 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > This is a non-functional change to avoid manual memory management. By > using a `std::string` instead of an explicitly heap-allocated buffer the > used pattern might even avoid hitting the heap altogether due to the SSO > but this function might be runtime bounded by I/O anyway. > > > Diffs > ----- > > 3rdparty/stout/include/stout/os/posix/mktemp.hpp > 8dab2599f13c3e1dab109423c8a938ec16540aaf > > > Diff: https://reviews.apache.org/r/70053/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
