> 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.
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()); ``` > On Oct. 20, 2016, 4:03 a.m., James Peach wrote: > > 3rdparty/stout/include/stout/os/posix/xattr.hpp, line 51 > > <https://reviews.apache.org/r/53041/diff/1/?file=1541992#file1541992line51> > > > > Add a flags argument? Darwin's ``XATTR_NOFOLLOW`` can be mapped to > > ``lgetxattr`` on Linux. > > Qian Zhang wrote: > Did you mean if caller passes `XATTR_NOFOLLOW` as flag to this method on > Linux, internally we call `lgetxattr()`? But I think it may confuse the > caller since on Linux `getxattr()` does not support flags at all, so I would > suggest not to mix `getxattr()` and `lgetxattr()` in one method. Yes, that makes sense. Later, you could add a ``lgetxattr`` wrapper. > 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? 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. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53041/#review153347 ----------------------------------------------------------- On Oct. 21, 2016, 9:29 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53041/ > ----------------------------------------------------------- > > (Updated Oct. 21, 2016, 9:29 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 > >
