----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11092/#review21947 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp <https://reviews.apache.org/r/11092/#comment45257> I would have actually expected os::pids to be an OS specific primitive for this stuff. See my comment below. 3rdparty/libprocess/3rdparty/stout/include/stout/os/ls.hpp <https://reviews.apache.org/r/11092/#comment45249> s/OS_HPP/OS_LS_HPP/ 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp <https://reviews.apache.org/r/11092/#comment45255> I'd really like to see this stuff simplified with os::sysctl. To help, I threw the review up at https://reviews.apache.org/r/11895. 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp <https://reviews.apache.org/r/11092/#comment45256> I feel like the OS X version of os::process(pid) should be implemented via 'proc_pidinfo' (basically the code above in os::processes()). In fact, just like the Linux implementation of os::processes(), it seems like the OS X implementation of os::processes() should invoke os::process(pid) for each pid it gets via sysctl. Better yet, it seems like os::processes() could be implemented OS agnostically via os::process(pid) and an OS specific os::pids(): ---------------------------- inline Try<std::list<Process> > processes() { const Try<std::set<pid_t> >& pids = os::pids(); if (pids.isError()) { return Error(pids.error()); } std::list<Process> result; foreach (pid_t pid, pids.get()) { const Try<os::Process>& process = os::process(pid); // Ignore any processes that disappear. if (process.isError()) { continue; } result.push_back(process.get()); } return result; } -------------- The Linux implementation of os::pids() would just invoke proc::pids() and the OS X implementation would look like this (using the new os::sysctl): -------------- Try<std::set<pid_t> > pids() { Try<int> maxproc = os::sysctl(CTL_KERN, KERN_MAXPROC).integer(); if (maxproc.isError()) { return Error(maxproc.error()); } Try<vector<kinfo_proc> > processes = os::sysctl(CTL_KERN, KERN_PROC, KERN_PROC_ALL).table(maxproc.get()); if (processes.isError()) { return Error(processes.error()); } std::set<pid_t> result; foreach (const kinfo_proc& process, processes.get()) { result.push_back(process.kp_proc.p_pid); } return result; } -------------- 3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp <https://reviews.apache.org/r/11092/#comment45254> Can we add a newline please? 3rdparty/libprocess/3rdparty/stout/include/stout/proc.hpp <https://reviews.apache.org/r/11092/#comment45248> s/tokens/format/ ? 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp <https://reviews.apache.org/r/11092/#comment45252> /s/linux/Linux/ and s/cpu/CPU/ ;) 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp <https://reviews.apache.org/r/11092/#comment45250> s/foreach(/foreach (/ 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp <https://reviews.apache.org/r/11092/#comment45253> Any reason not to 'break'? - Benjamin Hindman On June 11, 2013, 9:20 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11092/ > ----------------------------------------------------------- > > (Updated June 11, 2013, 9:20 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > These were reviewed by benh, and I'm sending these out in a new chain as the > old chain no longer made sense. > > Old reviews: > https://reviews.apache.org/r/10895/ > https://reviews.apache.org/r/10896/ > https://reviews.apache.org/r/10897/ > https://reviews.apache.org/r/10898/ > https://reviews.apache.org/r/10899/ > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/Makefile.am > 84062a0e1dfe4ec04bac7cac5ebaac4b945eb66e > 3rdparty/libprocess/3rdparty/stout/include/stout/fs.hpp > c1a05b58cbe1d0a9d006dec2db47d147149d421b > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 1b3fb47d7567b5467fef2a2bb15d5c4a2ea42aa5 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/ls.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp > PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/proc.hpp > b59735fbe993839aae3cb9ca7871517798c4cd46 > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp > 047778d05ebbbefd85e4a163dbb6ab8445edfb7f > 3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp > 2305ef53b700d5092802eab6f54a2893f5622e02 > > Diff: https://reviews.apache.org/r/11092/diff/ > > > Testing > ------- > > Added tests, make check on OSX and Linux. > > > Thanks, > > Ben Mahler > >
