mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Schlicht <...@mesosphere.io>
Subject Re: Review Request 45922: Added agent authorization flags.
Date Thu, 14 Apr 2016 11:51:35 GMT


> On April 13, 2016, 3:58 p.m., Benjamin Bannier wrote:
> > src/tests/cluster.cpp, lines 398-410
> > <https://reviews.apache.org/r/45922/diff/3/?file=1337119#file1337119line398>
> >
> >     You are shadowing the outer `Option<Authorizer*>` here with a `Result<Authorizer*>`.
All of the following assignments are to that and not the outer one. I believe this leads us
to assigning not what was intended in l.462 below. Having at least a different name would
help readability some here.
> >     
> >     As an aside, for my taste the control flow in this block has too much nesting
making this less transparent than it should be.

The shadowing has been resolved. If no authorizer and no ACLs were provided, a segfault could
occur. This has been fixed as well by making it an error.


- Jan


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


On April 14, 2016, 1 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> -----------------------------------------------------------
> 
> (Updated April 14, 2016, 1 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5142
>     https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c 
>   src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce 
>   src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea 
>   src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a 
>   src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
>   src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 
>   src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e 
>   src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896 
>   src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 
>   src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c 
>   src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 
> 
> Diff: https://reviews.apache.org/r/45922/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


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