mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhang...@cn.ibm.com>
Subject Re: Review Request 53041: Added `setxattr()` and `getxattr()` in stout.
Date Sat, 22 Oct 2016 12:27:50 GMT


> On Oct. 20, 2016, 12:03 p.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.

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__`.


> On Oct. 20, 2016, 12:03 p.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.
> 
> James Peach wrote:
>     Yes, that makes sense. Later, you could add a ``lgetxattr`` wrapper.

Yeah, I agree, we can add it later.


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53041/#review153347
-----------------------------------------------------------


On Oct. 21, 2016, 5:29 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53041/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2016, 5:29 p.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message