Re: Review Request 71976: Added support for systemd socket activation API.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/linux/systemd.cpp
Lines 346 (patched)


Can you declare and initilize in one go?

```
int flags = ...
```



src/linux/systemd.cpp
Lines 351-355 (patched)


Let's also declare and init in one go. Using a ternary operator shouldn't 
be too bad.



src/linux/systemd.cpp
Lines 384 (patched)


It might be helpful to rephrase the `Error` so we  can show `*listenPidEnv` 
in quotes.



src/linux/systemd.cpp
Lines 385 (patched)


No period at the end of this `Error`.



src/linux/systemd.cpp
Lines 402 (patched)


It might be helpful to rephrase the `Error` so we  can show `*listenPidEnv` 
in quotes.



src/linux/systemd.cpp
Lines 403 (patched)


No period at the end of this `Error`.



src/linux/systemd.cpp
Lines 408 (patched)


Remove period.



src/linux/systemd.cpp
Lines 414 (patched)


Can we add context here which fd we couldn't set cloexec for?

```
return Error("Could not set CLOEXEC for filedescriptor ...: + " 
cloexec.error()); 
```



src/linux/systemd.cpp
Lines 419 (patched)


Let's add spaces around `+` and a newline before `return`.



src/linux/systemd.cpp
Lines 458 (patched)


Can you document somewhere very prominently that this function is not safe 
to call from multiple threads since it mutates env?



src/linux/systemd.cpp
Lines 460-462 (patched)


Let's use `os::unsetenv` for "consistency".


- Benjamin Bannier


On Jan. 10, 2020, 2:48 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71976/
> ---
> 
> (Updated Jan. 10, 2020, 2:48 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for the systemd socket activation api,
> that allows systemd to pass listening file descriptors
> to a given service.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/linux/systemd.cpp 8126b31a542b62ce51cc6e0f6372986bffcc948b 
> 
> 
> Diff: https://reviews.apache.org/r/71976/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 71976: Added support for systemd socket activation API.

[Benno Evers]

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

Review request for mesos, Benjamin Bannier and Benjamin Mahler.


Repository: mesos


Description
---

Added support for the systemd socket activation api,
that allows systemd to pass listening file descriptors
to a given service.


Diffs
-

  src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
  src/linux/systemd.cpp 8126b31a542b62ce51cc6e0f6372986bffcc948b 


Diff: https://reviews.apache.org/r/71976/diff/1/


Testing
---


Thanks,

Benno Evers