Clean up based_io.c

[nrwahl2]

And some other assorted changes that came up along the way. This is part of a series of based refactors -- see #4011.

[nrwahl2]

These are tab-aligned. GitHub just isn't showing it correctly.

[nrwahl2]

Fixed a few little things and added one commit on top to drop some log messages.

@clumens FYI, even though this is 50 commits, it should be a pretty quick PR to review.

[nrwahl2]

The first 22 commits (at time of writing) are from #4000. I proactively rebased on those to resolve conflicts. No other changes, except for a commit to include action_relation_internal.h at the correct position.

[clumens]

According to 9be1f96accf:

9be1f96accf cib/io.c     (Andrew Beekhof    2011-09-02 09:52:25 +1000 652)         /* exit() could potentially affect the parent by closing things it shouldn't
9be1f96accf cib/io.c     (Andrew Beekhof    2011-09-02 09:52:25 +1000 653)          * Use _exit instead
9be1f96accf cib/io.c     (Andrew Beekhof    2011-09-02 09:52:25 +1000 654)          */

[nrwahl2]

I can clarify my comment, but I still don't see how _exit() is safer than exit(). From the exit(2) man page:

NOTES
       For a discussion on the effects of an exit, the transmission of exit 
status, zombie processes, signals sent, and so on, see exit(3).

       The function _exit() is like exit(3), but does not call any functions 
registered with atexit(3) or on_exit(3). Open stdio(3) streams are not flushed. 
On the other hand, _exit() does close open file descriptors, and this may cause 
an unknown delay, waiting for pending output to finish. If the delay is 
undesired, it may be useful to call functions like tcflush(3) before calling 
_exit().  Whether any pending I/O is canceled, and which pending I/O may be 
canceled upon _exit(), is implementation-dependent.

   C library/kernel differences
       The text above in DESCRIPTION describes the traditional effect of 
_exit(), which is to terminate a process, and these are the semantics specified 
by POSIX.1 and implemented by the C library wrapper function.  On modern 
systems, this means termination of all threads in the process.

       By contrast with the C library wrapper function, the raw Linux _exit() 
system call terminates only the calling thread, and actions such as reparenting 
child processes or sending SIGCHLD to the parent process are performed only if 
this is the last thread in the thread group.

       Up to glibc 2.3, the _exit() wrapper function invoked the kernel system 
call of the same name. Since glibc 2.3, the wrapper function invokes 
exit_group(2), in order to terminate all of the threads in a process.

Neither Pacemaker nor libqb calls atexit() or on_exit().

[clumens]

I should have been a little more clear. The code history gives us a reason why it could affect the parent, but doesn't explain what we're actually worried about could be closed. There are a handful of other places that call _exit but I imagine those are all just copy & paste or refactoring with not much thought given to why it's being used.

The only caller that gives a good reason is in lib/common/mock.c, but that's not going to be a concern here. I also imagine the caller in daemons/based/based_io.c would be pretty easy to test. In summary, I think we could probably just s/_exit/exit/, give it a cts-lab test, and hopefully get rid of these mysterious comments.

[nrwahl2]

...give it a cts-lab test...

Do you mean give it a new test, or do you mean run cts-lab? If you mean a new test, what do you have in mind for this?

[clumens]

I didn't notice if this happened over time or not... after so much change to based_io.c, it would be worth running it through include-what-you-use and cleaning up the include block at the top.

[nrwahl2]

Rebased on main, no other changes except "Include action_relation_internal.h at right spot" commit.

[nrwahl2]

I didn't notice if this happened over time or not... after so much change to based_io.c, it would be worth running it through include-what-you-use and cleaning up the include block at the top.

Agreed. I need to set up IWYU. I've had a tab open for it for a few days.

[nrwahl2]

Updated:

• Gave more detail in "TODO" within write_cib_async(), about _exit().
• Added commits on top to typedef the mainloop child callback and to clean up includes in based.