Re: Review Request 71866: Added new chmod() function to stout.

[Benno Evers]

> On Dec. 5, 2019, 12:43 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/os/posix/chmod.hpp
> > Lines 34 (patched)
> > 
> >
> > Let's return a `ErrnoError(errno)` here.
> > 
> > Do we need to include `cstdio`/`stdio.h` for `::strerror`?

We don't, `strerror()` should be defined in `string.h`/``.


- Benno


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


On Dec. 3, 2019, 6:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71866/
> ---
> 
> (Updated Dec. 3, 2019, 6:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new functions `os::chmod()` and `os::posix::chmod()` that
> are wrapping the POSIX `::chmod()` function.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/chmod.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/chmod.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/chmod.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71866/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71866: Added new chmod() function to stout.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/chmod.hpp
Lines 18-26 (patched)


We need to add a test for `os::chown` (disabled on Windows for now). Since 
stout is a header-only library even a trivial test is better than none.



3rdparty/stout/include/stout/os/posix/chmod.hpp
Lines 29 (patched)


Should we also include `sys/types.h` for `mode_t`?



3rdparty/stout/include/stout/os/posix/chmod.hpp
Lines 34 (patched)


Let's return a `ErrnoError(errno)` here.

Do we need to include `cstdio`/`stdio.h` for `::strerror`?


- Benjamin Bannier


On Dec. 3, 2019, 7:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71866/
> ---
> 
> (Updated Dec. 3, 2019, 7:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new functions `os::chmod()` and `os::posix::chmod()` that
> are wrapping the POSIX `::chmod()` function.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/chmod.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/chmod.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/chmod.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71866/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 71866: Added new chmod() function to stout.

[Benno Evers]

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

Review request for mesos, Benjamin Bannier and Benjamin Mahler.


Repository: mesos


Description
---

Added new functions `os::chmod()` and `os::posix::chmod()` that
are wrapping the POSIX `::chmod()` function.


Diffs
-

  3rdparty/stout/include/stout/os/chmod.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/chmod.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/chmod.hpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71866/diff/1/


Testing
---


Thanks,

Benno Evers