Re: Review Request 52153: Fixed openssl CA location logging.

[Benjamin Mahler]

> On Sept. 22, 2016, 7:58 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/openssl.cpp, line 493
> > 
> >
> > I recall that we prefer `.at(...)` even when there's an existence check 
> > directly preceeding the map access.
> 
> Benjamin Bannier wrote:
> I was briefly wondering why that would be prefered (terminate if key does 
> not exist?), but `map::at` being `const` sold me.

Ensuring const access is why I've been doing it, although it seems a bit 
worrying that `map::at` will throw an exception. If an exception is thrown 
within a `Process` execution context, libprocess will catch it and terminate 
the `Process`. After this point, dispatches to the terminated `Process` will be 
dropped, and we will only bail if it's the `Process` that main thread is 
waiting for. Ideally we guarantee that we will crash if a Process throws an 
exception.


- Benjamin


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


On Sept. 22, 2016, 9:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 22, 2016, 9:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Benjamin Bannier]

> On Sept. 22, 2016, 9:58 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/openssl.cpp, line 493
> > 
> >
> > I recall that we prefer `.at(...)` even when there's an existence check 
> > directly preceeding the map access.

I was briefly wondering why that would be prefered (terminate if key does not 
exist?), but `map::at` being `const` sold me.


- Benjamin


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


On Sept. 22, 2016, 11:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 11:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Benjamin Bannier]

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


Ship it!




modulo `map::operator[]` -> `map::at`.

- Benjamin Bannier


On Sept. 22, 2016, 11:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 11:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Joseph Wu]

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


Fix it, then Ship it!





3rdparty/libprocess/src/openssl.cpp (line 493)


I recall that we prefer `.at(...)` even when there's an existence check 
directly preceeding the map access.



3rdparty/libprocess/src/openssl.cpp (line 501)


Ditto here.


- Joseph Wu


On Sept. 22, 2016, 2:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 2:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Sept. 22, 2016, 9:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52031, 52033, 52153]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 22, 2016, 9:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 52153: Fixed openssl CA location logging.

[Till Toenshoff]

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

Review request for mesos, Joris Van Remoortere and Joseph Wu.


Bugs: MESOS-5320
https://issues.apache.org/jira/browse/MESOS-5320


Repository: mesos


Description
---

see summary.


Diffs
-

  3rdparty/libprocess/src/openssl.cpp c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 

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


Testing
---

make check


Thanks,

Till Toenshoff