Tidy up handling of intermediate pid fds

[smcv]

• Use PIPE_READ_END, PIPE_WRITE_END to clarify use of a socketpair

  Both sockets in the socket pair are technically bidirectional, but we
  are using them as though they were unidirectional, with the same
  convention as pipe(): the first socket in the array is used for reading,
  and the second is for writing.

• Close unused ends of intermediate_pids_sockets sooner

  Instead of making this conditional and keeping track of the correct
  condition under which to call it, we can use cleanup_fdp(), which is
  a no-op when called with a pointer to a negative number, to close the
  socket unconditionally.

  In the parent bwrap monitor process (outside the sandbox), we never
  want to use the write end (which is reserved for the child), so we
  can and should close it as soon as we have forked.

  Conversely, in the child process, we never want to use the read end
  (which is reserved for the parent), so we should close that as soon
  as we know we are in the child.

• Use cleanup_fd to close intermediate pid sockets

  This closes the fd and sets the variable to -1 as a single operation,
  which is easier to reason about because it does not leave any variables
  containing dangling references to invalid fds.

---

cc @refi64 @WGH- @mcatanzaro

I think #576 would have better clarity if it was rebased on this and used similar techniques. Reviews welcome, even from non-maintainers.

[cgzones]

What about assigning -1 to the passe pointer in cleanup_fdp(), so a potential second close() call won't close an uninvolved descriptor.

For the cleanup attribute annotated variables compilers should be able to optimize the assignment out.

Another unrelated thought: systemd uses a safe_close() wrapper instead of directly close(3) to assert EBADF was not returned, to spot file descriptor mixups, see https://github.com/systemd/systemd/blob/v256.7/src/basic/fd-util.c#L59.

[smcv]

What about assigning -1 to the passe pointer in cleanup_fdp()

Oh... I thought it already did that, but in fact it doesn't, so this PR needs redoing with either a different function or an augmented version of cleanup_fdp() (as do #666 and #667).

[smcv]

This closes the fd and sets the variable to -1 as a single operation

[citation needed]

[smcv]

Another unrelated thought: systemd uses a safe_close() wrapper instead of directly close(3) to assert EBADF was not returned, to spot file descriptor mixups,

GLib does similarly, in g_close(). A potential problem with that technique is that the assertion is not going to be async-signal-safe, which is troublesome in a process that forks. On the other hand, bubblewrap is single-threaded, so it's usually OK for us to call non-async-signal-safe functions after fork().