> On Oct. 20, 2016, 4:03 a.m., James Peach wrote: > > 3rdparty/stout/include/stout/os/posix/xattr.hpp, line 24 > > <https://reviews.apache.org/r/53041/diff/1/?file=1541992#file1541992line24> > > > > Since the flags differ between platform, it might be a good idea to > > define stout-specific versions. > > Qian Zhang wrote: > Can you elaborate on stout-specific versions? Did you mean we define two > versions of setxattr() here, one for Linux and another for APPLE? > > James Peach wrote: > I meant that the flags differ across the two platforms. So the caller has > to know what flags are safe to pass for both. For example, the API doesn't > prevent you trying to pass ``XATTR_NOFOLLOW`` on Linux, which won't build. > I'm wondering whether stout ought to define its own versions of the flags so > that you can more easily write portable code using this API. > > Qian Zhang wrote: > I'd like to expose the native OS flags to caller and let the caller to > decides what kind of flags should be used, e.g., if caller pass > `XATTR_NOFOLLOW` on Linux and find it can not built, then the caller should > put its code under `__APPLE__`.
It is a pretty poor abstraction if the caller has to ifdef. But since that is not needed now AFAICT, it doesn't matter much. > On Oct. 20, 2016, 4:03 a.m., James Peach wrote: > > 3rdparty/stout/include/stout/os/posix/xattr.hpp, line 64 > > <https://reviews.apache.org/r/53041/diff/1/?file=1541992#file1541992line64> > > > > You could use ``std::vector<char>`` here to avoid manual memory > > management. > > Qian Zhang wrote: > Can you please let me know the advantage of std::vector<char>? To me, new > + memset can satisfy my requirement here. > > James Peach wrote: > Dropped the issue since this is just a suggestion :) > > IMHO, using vector<char> is more robust since you get RAII behaviour and > don't need to explicitly delete on the error path, eg. > > ``` > std::vector<char> buffer(size); > getxattr(path.c_str(), name.c_str(), &buffer[0], size, 0, 0); > return std:string(buffer.begin(), buffer.size()); > > ``` Not sure that ``errno`` is guaranteed to be preserved across the ``delete``. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53041/#review153347 ----------------------------------------------------------- On Oct. 23, 2016, 3:01 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53041/ > ----------------------------------------------------------- > > (Updated Oct. 23, 2016, 3:01 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6360 > https://issues.apache.org/jira/browse/MESOS-6360 > > > Repository: mesos > > > Description > ------- > > Added `setxattr()` and `getxattr()` in stout. > > > Diffs > ----- > > 3rdparty/stout/include/Makefile.am 1eb9c146d3eebacaea32e0f724d145f0c1dd676f > 3rdparty/stout/include/stout/os.hpp > 96e8621b198a3ec4cce78e0a6ff5f271eda05ff1 > 3rdparty/stout/include/stout/os/posix/xattr.hpp PRE-CREATION > 3rdparty/stout/include/stout/os/windows/xattr.hpp PRE-CREATION > 3rdparty/stout/include/stout/os/xattr.hpp PRE-CREATION > > Diff: https://reviews.apache.org/r/53041/diff/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >
