Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Jan. 12, 2016, 12:46 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 12, 2016, 12:46 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3640
> https://issues.apache.org/jira/browse/MESOS-3640
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `stout/os/windows/ls.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/ls.hpp 
> 7be7d123c31c562148c50e843fbf709de73a9133 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
> fcc6986f1b6355e0f0f7e25a6e428ab878a97ee4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/direntsize.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
> 5b41344ead115d14dcee8c87a63ed647002f9aae 
> 
> Diff: https://reviews.apache.org/r/39802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Alex Clemmer]

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

(Updated Jan. 12, 2016, 12:46 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3640
https://issues.apache.org/jira/browse/MESOS-3640


Repository: mesos


Description
---

Windows: Implemented `stout/os/windows/ls.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/ls.hpp 
7be7d123c31c562148c50e843fbf709de73a9133 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
fcc6986f1b6355e0f0f7e25a6e428ab878a97ee4 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/direntsize.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
5b41344ead115d14dcee8c87a63ed647002f9aae 

Diff: https://reviews.apache.org/r/39802/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Joris Van Remoortere]

> On Jan. 11, 2016, 12:38 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp, lines 
> > 36-42
> > 
> >
> > If this is the only section that is different between the POSIX and 
> > windows version, does it make sense to factor out this size calculation, 
> > and leave a common `ls` implementation?
> 
> Alex Clemmer wrote:
> It might. Lets look at a couple ways of doing this:
> 
> * Since the main problem is `fpathconf` (which doesn't make much sense on 
> Windows), we could create a new function, `os::fpathconf` that simply always 
> returns `-1`.
> * We could create some other function whose job is just to give the size 
> of the `dirent` struct on a particular platform. But, I don't know what to 
> call it, or where to put it.
> * Just keep the forked version.
> 
> I'm not sure if any of these really reaches out to you. For me,  I liked 
> the last one because I felt it was clearest on both platforms. For example, 
> the `os::fpathconf` approach will be a lot less clear because it won't be 
> obvious to _anyone_ that we're just returning -1 all the time, or what the 
> consequences is for the reported dirent struct size on Windows. The forked 
> dirent-size-function is not elegant, and adds API bloat, and still requires a 
> forked file, albeit a much smaller one.
> 
> I don't know, I'm open to other opinions. I think they all have serious 
> downsides. My second choice is the forked dirent-size-function option, 
> because it doesn't sacrifice code readability.

How about `os::dirent_size`?


- Joris


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


On Jan. 11, 2016, 11:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 11, 2016, 11:59 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/posix/ls.hpp 
> fcc6986f1b6355e0f0f7e25a6e428ab878a97ee4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
> 5b41344ead115d14dcee8c87a63ed647002f9aae 
> 
> Diff: https://reviews.apache.org/r/39802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Alex Clemmer]

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

(Updated Jan. 12, 2016, 12:40 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3640
https://issues.apache.org/jira/browse/MESOS-3640


Repository: mesos


Description
---

Windows: Implemented `stout/os/windows/ls.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/ls.hpp 
7be7d123c31c562148c50e843fbf709de73a9133 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
fcc6986f1b6355e0f0f7e25a6e428ab878a97ee4 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/direntsize.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
5b41344ead115d14dcee8c87a63ed647002f9aae 

Diff: https://reviews.apache.org/r/39802/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Alex Clemmer]

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

(Updated Jan. 12, 2016, 12:39 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 (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/ls.hpp 
7be7d123c31c562148c50e843fbf709de73a9133 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
fcc6986f1b6355e0f0f7e25a6e428ab878a97ee4 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/direntsize.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
5b41344ead115d14dcee8c87a63ed647002f9aae 

Diff: https://reviews.apache.org/r/39802/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Alex Clemmer]

> On Jan. 9, 2016, 1:05 a.m., Yi Sun wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp, line 55
> > 
> >
> > So the entry is always pointing to the allocated memory when success.

Following up from our Slack conversation where I clarified your question: 
`entry` actually points at the current directory entry of `dir`, so a call to 
free is not necessary here.


- Alex


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


On Jan. 4, 2016, 11:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 4, 2016, 11:20 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/posix/ls.hpp 
> 7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
>   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`.

[Alex Clemmer]

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

(Updated Jan. 11, 2016, 11:59 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 (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
fcc6986f1b6355e0f0f7e25a6e428ab878a97ee4 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
5b41344ead115d14dcee8c87a63ed647002f9aae 

Diff: https://reviews.apache.org/r/39802/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Alex Clemmer]

> On Jan. 11, 2016, 12:38 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp, lines 
> > 36-42
> > 
> >
> > If this is the only section that is different between the POSIX and 
> > windows version, does it make sense to factor out this size calculation, 
> > and leave a common `ls` implementation?

It might. Lets look at a couple ways of doing this:

* Since the main problem is `fpathconf` (which doesn't make much sense on 
Windows), we could create a new function, `os::fpathconf` that simply always 
returns `-1`.
* We could create some other function whose job is just to give the size of the 
`dirent` struct on a particular platform. But, I don't know what to call it, or 
where to put it.
* Just keep the forked version.

I'm not sure if any of these really reaches out to you. For me,  I liked the 
last one because I felt it was clearest on both platforms. For example, the 
`os::fpathconf` approach will be a lot less clear because it won't be obvious 
to _anyone_ that we're just returning -1 all the time, or what the consequences 
is for the reported dirent struct size on Windows. The forked 
dirent-size-function is not elegant, and adds API bloat, and still requires a 
forked file, albeit a much smaller one.

I don't know, I'm open to other opinions. I think they all have serious 
downsides. My second choice is the forked dirent-size-function option, because 
it doesn't sacrifice code readability.


- Alex


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


On Jan. 4, 2016, 11:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 4, 2016, 11:20 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/posix/ls.hpp 
> 7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
>   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`.

[Joris Van Remoortere]

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 


Let's add the preservation comment here as well.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (lines 36 - 
42)


If this is the only section that is different between the POSIX and windows 
version, does it make sense to factor out this size calculation, and leave a 
common `ls` implementation?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (line 45)


let's put backticks around malloc.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (line 66)


Let's add a comment here like you did for malloc:
`Preserve `readdir_r` error.`


- Joris Van Remoortere


On Jan. 4, 2016, 11:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 4, 2016, 11:20 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/posix/ls.hpp 
> 7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
>   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`.

[Yi Sun]

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

Ship it!


Ship It!


3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (line 55)


So the entry is always pointing to the allocated memory when success.


- Yi Sun


On Jan. 4, 2016, 11:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 4, 2016, 11:20 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/posix/ls.hpp 
> 7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
>   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`.

[Alex Naparu]

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

Ship it!


Ship It!

- Alex Naparu


On Jan. 4, 2016, 11:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 4, 2016, 11:20 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/posix/ls.hpp 
> 7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
>   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`.

[Alex Clemmer]

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

(Updated Jan. 4, 2016, 11:18 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/posix/ls.hpp 
7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
  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`.

[Alex Clemmer]

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

(Updated Jan. 4, 2016, 11:20 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/posix/ls.hpp 
7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
  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`.

[Alex Clemmer]

> On Nov. 2, 2015, 10:02 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp, lines 
> > 66-69
> > 
> >
> > 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
> 
>

Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

[Alex Clemmer]

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

(Updated Nov. 17, 2015, 7:02 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 (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
  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`.

[Alex Clemmer]

---
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 (updated)
-

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

[Joseph Wu]

---
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)


Add `#include `?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp (line 22)


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)


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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39537, 39538, 39539, 39540, 39541, 39383, 39559, 39219, 
39560, 39583, 39584, 39620, 39621, 39622, 39623, 39019, 39802]

All tests passed.

- Mesos ReviewBot


On Oct. 30, 2015, 6:46 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Oct. 30, 2015, 6:46 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
> 
>