src/sim/process.cc (Diff revision 1)

This should be a fatal. 
In fact, maybe 2. One to check that it's less than MAX_PID, and another to make sure it's not already in use.

Show all issues

src/sim/system.hh (Diff revision 1)

Can this just be an std::set<int>? This is only used to make sure there are no duplicates, right?

Show all issues

1. Brandon Potter Nov. 14, 2016, 11:07 a.m. (Nov. 14, 2016, 11:07 a.m.)

   Why is the std::set<int> preferred over std::array<bool>?
   Yes, it's used to figure out which PIDs are available and which have already been claimed by other processes.

2. Jason Lowe-Power Nov. 14, 2016, 11:12 a.m. (Nov. 14, 2016, 11:12 a.m.)

   I think a std::set<int> is much more clear as to what it is tracking, and it is simpler to access in many cases (e.g., it has a size() function).
   Plus, it's smaller. If you only have 1 PID (the most common case), then you only have a single entry in the set, comparted to 32768... well, it could be bits, but likely it's at least chars. I can't find any information as to the implementation of std::array<bool>.

3. Brandon Potter Nov. 14, 2016, 11:48 a.m. (Nov. 14, 2016, 11:48 a.m.)

   OK, I made it into a set. It'll require a change to the clone-exec patch as well, but I'll make that and post it now.

src/sim/syscall_emul.cc (Diff revision 1)

Use nullptr throughout.

Show all issues

Minor issue, otherwise it LGTM, assuming you address Jason's comments as well.

Ship It!

Thanks for the change!