mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 52154: Avoided modifying process environment.
Date Mon, 26 Sep 2016 16:39:27 GMT


> On Sept. 22, 2016, 2:15 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp, lines 317-318
> > <https://reviews.apache.org/r/52154/diff/1/?file=1508026#file1508026line317>
> >
> >     Can we have a short comment here explaining that `load` overload? Does it still
even try to check the process environment or is it totally relying on that map being a complete
environment view?

Thanks for bringing this up. I went and looked more carefully at how
`FlagsBase::extract` is implemented and found that it actually does some
perform some slightly specific magic where the case of environment variables
does partly ignore case. I prefixed this review with
https://reviews.apache.org/r/52256/ which exposes `FlagsBase::extract` so we
can avoid reimplementing this code here.


- Benjamin


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


On Sept. 26, 2016, 6:16 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52154/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Till Toenshoff.
> 
> 
> Bugs: MESOS-6216
>     https://issues.apache.org/jira/browse/MESOS-6216
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This code modified the process environment in order to support upgrading
> on the fly from old-style libprocess SSL variables `SSL_` to
> `LIBPROCESS_SSL_`. Modifying the process environment at this point is
> not safe as other actors might concurrently read out that same
> environment.
> 
> Instead avoid changing the process environment altogether since flags
> can just as well be read from a map.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/openssl.cpp c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52154/diff/
> 
> 
> Testing
> -------
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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