Don't clobber td->td_retval[0] in proc_reap().

[ed]

Summary

While writing tests for CloudABI, I noticed that close() on process descriptors returns the process ID of the child process. This is interesting, as close() is only allowed to return 0 or -1. It turns out that we clobber td->td_retval[0] in proc_reap(), so that wait*() properly returns the process ID.

Change proc_reap() to leave td->td_retval[0] alone. Set the process ID in kern_wait6() instead, by keeping track of the PID before we (potentially) reap the process.

Test Plan

CloudABI unit tests pass. FreeBSD wait*() still seems to work; the system works fine.

[kib]

Overall it is fine, just consider my minor note above.

sys/kern/kern_exit.c

842 ↗

(On Diff #6807)

This is fine.

1193 ↗

(On Diff #6807)

But I do not like this part of the change. IMO the explicit placement of the assignment for each return branch would make both your goal and code cleaner, rather then finding yet another 'common' place.

From what I see, this means that the line commented should move to the block with 'return 0' right below the proc_to_reap() call. Everything else is already handled by the method I see as cleaner.

[ed]

sys/kern/kern_exit.c

1193 ↗

(On Diff #6812)

Yes, I can see what you're hinting at. Makes sense!

We do need to keep track of the PID now, as proc_reap() might destroy the process structure. Let's keep track of the PID in a new variable pid and set it right before we return (0);. Does this look all right?