Re: [PATCH v3 06/10] run-command: add clean_on_exit_handler

> On 30 Jul 2016, at 11:50, Johannes Sixt <j6t@kdbg.org> wrote:
> 
> Am 30.07.2016 um 01:37 schrieb larsxschneider@gmail.com:
>> Some commands might need to perform cleanup tasks on exit. Let's give
>> them an interface for doing this.
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> run-command.c | 12 ++++++++----
>> run-command.h |  1 +
>> 2 files changed, 9 insertions(+), 4 deletions(-)
>> 
>> diff --git a/run-command.c b/run-command.c
>> index 33bc63a..197b534 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -21,6 +21,7 @@ void child_process_clear(struct child_process *child)
>> 
>> struct child_to_clean {
>> 	pid_t pid;
>> +	void (*clean_on_exit_handler)(pid_t);
>> 	struct child_to_clean *next;
>> };
>> static struct child_to_clean *children_to_clean;
>> @@ -30,6 +31,8 @@ static void cleanup_children(int sig, int in_signal)
>> {
>> 	while (children_to_clean) {
>> 		struct child_to_clean *p = children_to_clean;
>> +		if (p->clean_on_exit_handler)
>> +			p->clean_on_exit_handler(p->pid);
> 
> This summons demons. cleanup_children() is invoked from a signal handler. In this case, it can call only async-signal-safe functions. It does not look like the handler that you are going to install later will take note of this caveat!
> 
>> 		children_to_clean = p->next;
>> 		kill(p->pid, sig);
>> 		if (!in_signal)
> 
> The condition that we see here in the context protects free(p) (which is not async-signal-safe). Perhaps the invocation of the new callback should be skipped in the same manner when this is called from a signal handler? 507d7804 (pager: don't use unsafe functions in signal handlers) may be worth a look.