Re: Review Request 53706: Implemented `os::user' on Windows.

[Joseph Wu]

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




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


It would be nice to have a comment here explaining:

* We include `Security.h` for the `GetUserNameEx` method included by 
`SecExt.h`.  Comments in `SecExt.h` note that `SecExt.h` should only be 
included via `Security.h`.
* In order to include `SecExt.h`, we must define either `SECURITY_WIN32` or 
`SECURITY_KERNEL`.  These determine whether the methods are usermode or kernel 
operations.  We don't need the kernel level permissions.



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


Instead of adding a library here, can't we add this to the `STOUT_LIBS` 
variable?



3rdparty/stout/include/stout/os/windows/su.hpp (line 63)


Let's use a `while (true)` instead.  Also see the next issue.



3rdparty/stout/include/stout/os/windows/su.hpp (lines 66 - 70)


According to 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms724435(v=vs.85).aspx

When this error value is set:
> The lpNameBuffer buffer is too small. The lpnSize parameter contains the 
number of bytes required to receive the name.

That means you only need to resize the buffer once.  This also means you 
can remove the loop.



3rdparty/stout/include/stout/os/windows/su.hpp (line 71)


Instead of effectively calling `::GetLastError` twice (once directly, once 
via `WindowsError`), just save the value of `WindowsError` and access the code 
via `.code`.

Also, it would be nice to include some more text in the error body.



3rdparty/stout/include/stout/os/windows/su.hpp (lines 76 - 77)


Can the `auto ret` be inlined into the second line?

Also, what's the importance behind replacing backslashes with dots?


- Joseph Wu


On Nov. 29, 2016, 11:54 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53706/
> ---
> 
> (Updated Nov. 29, 2016, 11:54 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, 
> Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `os::user' on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/su.hpp 
> 1bb70964adbb80aa6502fbfe69de2c34dc74e655 
> 
> Diff: https://reviews.apache.org/r/53706/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 53706: Implemented `os::user' on Windows.

[Daniel Pravat]

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

(Updated Nov. 30, 2016, 7:54 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Implemented `os::user' on Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/su.hpp 
1bb70964adbb80aa6502fbfe69de2c34dc74e655 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 53706: Implemented `os::user' on Windows.

[Joseph Wu]

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




3rdparty/stout/include/stout/os/windows/su.hpp (lines 61 - 63)


Looks like a rebasing artifact snuck into your review.


- Joseph Wu


On Nov. 22, 2016, 10:30 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53706/
> ---
> 
> (Updated Nov. 22, 2016, 10:30 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, 
> Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `os::user' on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/su.hpp 
> 1bb70964adbb80aa6502fbfe69de2c34dc74e655 
> 
> Diff: https://reviews.apache.org/r/53706/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 53706: Implemented `os::user' on Windows.

[Daniel Pravat]

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

(Updated Nov. 23, 2016, 6:30 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Implemented `os::user' on Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/su.hpp 
1bb70964adbb80aa6502fbfe69de2c34dc74e655 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 53706: Implemented `os::user' on Windows.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Implemented `os::user' on Windows.


Diffs
-

  3rdparty/stout/include/stout/os/windows/su.hpp 
777140e1139d6eeab20780e8c0d0a273ce6a8125 

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


Testing
---


Thanks,

Daniel Pravat