You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2015/11/02 23:02:44 UTC
Re: Review Request 39802: Windows: Implemented
`stout/os/windows/ls.hpp`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39802/#review104806
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (lines 20 - 21)
<https://reviews.apache.org/r/39802/#comment163051>
Add `#include <stout/error.hpp>`?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (line 22)
<https://reviews.apache.org/r/39802/#comment163052>
I might be blind, but I don't see where this is used.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (lines 66 - 69)
<https://reviews.apache.org/r/39802/#comment163049>
This seems weird. `readdir_r`, per your implementation, only returns 0 or 1. But in the error case, you set `errno` to `EBADF` within `readdir_r`.
I'm guessing you'll want to "preserve" the error, just like you did above for malloc.
- Joseph Wu
On Oct. 29, 2015, 11:46 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 11:46 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `stout/os/windows/ls.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 5b6fba13ce215af5801fd0867f6e774e100689ca
>
> Diff: https://reviews.apache.org/r/39802/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39802: Windows: Implemented
`stout/os/windows/ls.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Nov. 2, 2015, 10:02 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp, lines 66-69
> > <https://reviews.apache.org/r/39802/diff/1/?file=1112947#file1112947line66>
> >
> > This seems weird. `readdir_r`, per your implementation, only returns 0 or 1. But in the error case, you set `errno` to `EBADF` within `readdir_r`.
> >
> > I'm guessing you'll want to "preserve" the error, just like you did above for malloc.
This is because the POSIX implementation does this. I think we should stay consistent, so recommend either changing both or changing neither.
For now I'll change it in both implementations, but does changing this affect downstream? For example, if we change this, will it break logging infrastructure somewhere?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39802/#review104806
-----------------------------------------------------------
On Nov. 16, 2015, 9:14 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2015, 9:14 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `stout/os/windows/ls.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 5b6fba13ce215af5801fd0867f6e774e100689ca
>
> Diff: https://reviews.apache.org/r/39802/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>