mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Clemmer <clemmer.alexan...@gmail.com>
Subject Re: Review Request 47168: Windows: Implement `kill` and `killtree`.
Date Tue, 10 May 2016 18:00:03 GMT


> On May 10, 2016, 5:41 p.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp, line 30
> > <https://reviews.apache.org/r/47168/diff/1/?file=1377773#file1377773line30>
> >
> >     If we don't mean to expose `kill_process` here, can we please put it in the
`internal::os::` namespace?
> >     
> >     Beyond the fact that I want to keep the `os::` namespace clean, it's important
to note that out `pid_t` is unsigned, while most POSIX implementations are signed. So if we
expose a lot of APIs that take `pid_t` as argument, people could expect that they take negative
numbers (as, for example, `kill` does). I realize this is a programming surface that exists
only on Windows, but one of the ways I want to limit risk is by really buckling down on APIs
that expose some form of `pid_t` because all of those uses could lead to risky behavior, so
it's best to have as few of them floating around as we can, in general. So relative to the
amount of work here, I think the payoff is high.

Sorry, that's `os::internal::`. My bad.


- Alex


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


On May 10, 2016, 8:54 a.m., Daniel Pravat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47168/
> -----------------------------------------------------------
> 
> (Updated May 10, 2016, 8:54 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and
Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Implement `kill` and `killtree`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 33ddb06e25920096f2d16d4f372129ee2f6a8721

>   3rdparty/libprocess/3rdparty/stout/include/stout/os/kill.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/kill.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 3828c53c0dbbf370d642189998af75b9af434e9d

> 
> Diff: https://reviews.apache.org/r/47168/diff/
> 
> 
> Testing
> -------
> 
> Windows: build/test w/ Marathon
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message