> On Nov. 4, 2014, 3:09 a.m., Adam B wrote: > > Ship It! > > Adam B wrote: > Following the logic with my internal C++ compiler/interpreter (aka my > brain), it's pretty clear that we would have returned earlier in this > function if framework were in fact NULL, so the null-check branch is indeed > unreachable code. > > Ben Mahler wrote: > Definitely unreachable, I just took a glance and couldn't easily figure > out when this case occurs. I also couldn't quickly figure out whether the > creation of the framework was something we want to do in this case vs. log a > warning and bail. If we bail when the framework is NULL and this is something > we expect to occur, do we need to send task lost? > > With these questions, I realized I should just let vinod review this > given he had more context around this.
Vinod and I discussed at some length in https://reviews.apache.org/r/23912 which style of dealing with the framework life cycle to pursue. The code in question here is an artefact of the style I initially opted for: relatively late destruction of the framework. You would then have to check if it still exists as done by the now dead code. With Vinod's now implemented approach, the framework gets discarded at the earliest opportunity. This enabled the test at the top that made the code in question dead. This larger problem is that the original code hinted at BOTH styles. I conjecture that this is not an isolated incident of such latent confusion. - Bernd ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27567/#review59760 ----------------------------------------------------------- On Nov. 4, 2014, 3:46 p.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27567/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2014, 3:46 p.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-2038 > https://issues.apache.org/jira/browse/MESOS-2038 > > > Repository: mesos-git > > > Description > ------- > > Removed a few lines of dead code that coverty discovered. > > > Diffs > ----- > > src/slave/slave.cpp 5e9b0e4f93a5100a91340e1f6fb1fe160b2eea4b > > Diff: https://reviews.apache.org/r/27567/diff/ > > > Testing > ------- > > none. > expecting/waiting for review bot to report no problem. > > > Thanks, > > Bernd Mathiske > >
