> On July 25, 2016, 2:03 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, lines 47-63 > > <https://reviews.apache.org/r/49849/diff/4/?file=1450015#file1450015line47> > > > > I would suggest to slightly change these code to: > > ``` > > Subsystem* subsystem = nullptr; > > > > if (_name == CGROUP_SUBSYSTEM_CPU_NAME) { > > subsystem = new CpuSubsystem(_flags, _hierarchy); > > } else { > > return Error("Unknown subsystem"); > > } > > > > Try<Nothing> load = subsystem->load(); > > if (load.isError()) { > > return Error("Failed to load subsystem '" + _name + "': " + > > load.error()); > > } > > > > return Owned<Subsystem>(subsystem); > > ``` > > haosdent huang wrote: > It may cause a memory leak at here if change to avoid code. > > ``` > Try<Nothing> load = subsystem->load(); > if (load.isError()) { > -> return Error("Failed to load subsystem '" + _name + "': " + > load.error()); > } > ``` > > Qian Zhang wrote: > Sorry, my bad :-( > > Then what about this? > ``` > Owned<Subsystem> subsystem; > > if (_name == CGROUP_SUBSYSTEM_CPU_NAME) { > subsystem = Owned<Subsystem>(new CpuSubsystem(_flags, _hierarchy)); > } else { > return Error("Unknown subsystem"); > } > > Try<Nothing> load = subsystem->load(); > if (load.isError()) { > return Error("Failed to load subsystem '" + _name + "': " + > load.error()); > } > > return subsystem; > ``` > I just want to avoid two local variabls (`_subsystem` and `subsystem`) in > this method :-)
oh, I see. Let me remove `_subsystem` according your comment. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49849/#review143334 ----------------------------------------------------------- On July 25, 2016, 2:48 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49849/ > ----------------------------------------------------------- > > (Updated July 25, 2016, 2:48 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha. > > > Bugs: MESOS-5042 > https://issues.apache.org/jira/browse/MESOS-5042 > > > Repository: mesos > > > Description > ------- > > Implemented `CpuSubsystem`. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp > 4a9f55bf3b217405bf90943f27a976422877a99e > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp > 5f52a076a1fa3a21d886cb961ddeed5046a38d7c > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp > a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 > > Diff: https://reviews.apache.org/r/49849/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
