Re: Review Request 43985: Windows: [1/3] Implemented `sendfile`.

2016-04-20 Thread Daniel Pravat 


> On April 19, 2016, 10:21 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp, 
> > line 45
> > 
> >
> > `s/send/sent/`

Thanks!


- Daniel


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


On April 16, 2016, 1:10 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43985/
> ---
> 
> (Updated April 16, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: [1/3] Implemented `sendfile`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
> 293f82f6730551491504721e0af28e9537540db1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 
> f3197679a9c22b37acae003262e5c6d21e110f56 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 750ca6749cb028703ed2fb5dec2aac6c5dabea0d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> a7f855dc9d0a87fe3b6d1611e7ae22e4d7cd7b6d 
> 
> Diff: https://reviews.apache.org/r/43985/diff/
> 
> 
> Testing
> ---
> 
> Build Windows,OSX
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 43985: Windows: [1/3] Implemented `sendfile`.

2016-04-19 Thread Michael Park 

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




3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (lines 
33 - 34)


(1) `s/On error, On error,/On error,/`
(2) Remove the extra space in `//  Try`. i.e., `// Try`



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (lines 
39 - 40)


```
inline Try sendfile(
int s, int fd, off_t offset, size_t length)
{
```



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (line 45)


`s/send/sent/`



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (line 46)


(1) `s/return return/return/`
(2) Based on other syscalls, it seems like we should just do `return 
SocketError();`. e.g., `close.hpp`, `chroot.hpp`, `fnctl.hpp`, etc. Here and 
below.



3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp (lines 16 - 17)


When we fork to have separate implementation for `POSIX` and `Windows`, we 
should keep them completely separate. That is, even though `` 
is used in both implementations, they should include them individually.

For example, starting from 
`3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp` we 
can't find where `ErrnoError` came from.



3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp (lines 19 - 24)


Let's consolidate this with the `process::network::SocketError` in 
`3rdparty/libprocess/include/process/network.hpp`.

For now, I think `stout/error.hpp` is the best place to put it.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (lines 
16 - 17)


Add a newline in between.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
23)


Backquotes around `Try`.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (lines 
24 - 25)


```
inline Try sendfile(
int s, int fd, off_t offset, size_t length)
{
```



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
27)


`s/ will/, it will/`



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (lines 
28 - 30)


Please use the correct C++ casts here for the casts to `HANDLE`, `LONG`.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
30)


`hight_Part`? did we mean `high_part`? Just `high` seems fine too.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
31)


It looks like the second parameter of `SetFilePointer` is `LONG`, not 
`DWORD`. Why do we cast to `DWORD`? Also, please use C++ cast.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
33)


Should return `SocketError` here (I know they're the same, but still).



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
39)


Should return `SocketError` here (I know they're the same, but still).


- Michael Park


On April 16, 2016, 1:10 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43985/
> ---
> 
> (Updated April 16, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: [1/3] Implemented `sendfile`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
> 293f82f6730551491504721e0af28e9537540db1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 
> f3197679a9c22b37acae003262e5c6d21e110f56 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 750ca6749cb028703ed2fb5dec2aac6c5dabea0d 
>

Re: Review Request 43985: Windows: [1/3] Implemented `sendfile`.

2016-04-15 Thread Daniel Pravat 

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

(Updated April 16, 2016, 1:10 a.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: [1/3] Implemented `sendfile`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
293f82f6730551491504721e0af28e9537540db1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 
f3197679a9c22b37acae003262e5c6d21e110f56 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp 
750ca6749cb028703ed2fb5dec2aac6c5dabea0d 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
a7f855dc9d0a87fe3b6d1611e7ae22e4d7cd7b6d 

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


Testing (updated)
---

Build Windows,OSX


Thanks,

Daniel Pravat

Re: Review Request 43985: Windows: [1/3] Implemented `sendfile`.

2016-04-15 Thread Daniel Pravat 

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

(Updated April 16, 2016, 12:19 a.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Michael Park.


Summary (updated)
-

Windows: [1/3] Implemented `sendfile`.


Repository: mesos


Description (updated)
---

Windows: [1/3] Implemented `sendfile`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
293f82f6730551491504721e0af28e9537540db1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp 
750ca6749cb028703ed2fb5dec2aac6c5dabea0d 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
a7f855dc9d0a87fe3b6d1611e7ae22e4d7cd7b6d 

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


Testing
---

Build Windows,OSX
Run Windows


Thanks,

Daniel Pravat