Re: [PATCH] tmp-objdir: do not opendir() when handling a signal

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 27, 2022, 16:50 UTC
Message-ID: <xmqqedvw7od9.fsf@gitster.g>
In-Reply-To: <YzLiI1HZeBszsIJq@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> So all of that makes me think we should not add a new flag here, but
> instead just avoid calling the function entirely from
> tmp_objdir_destroy_1().

Thanks. I missed that undefined access to errno that breaks the intention of the patch.

Show 24 quoted lines

> But then we can observe that tmp_objdir_destroy_1() is basically doing
> nothing if on_signal is set. So there is really no point in setting up
> the signal handler at all. We should just set up the atexit() handler.
> I.e., something like:
>
> diff --git a/tmp-objdir.c b/tmp-objdir.c
> index a8be92bca1..10549e95db 100644
> --- a/tmp-objdir.c
> +++ b/tmp-objdir.c
> @@ -169,7 +169,6 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix)
>  	the_tmp_objdir = t;
>  	if (!installed_handlers) {
>  		atexit(remove_tmp_objdir);
> -		sigchain_push_common(remove_tmp_objdir_on_signal);
>  		installed_handlers++;
>  	}
>  
>
> with the commit message explaining that we can't do the cleanup in a
> portable and signal-safe way, so we just punt on the whole concept.
>
> There's also some minor cleanup we could do elsewhere to drop the
> "on_signal" argument (which can come as part of the same patch, or on
> top).

;-)  I like the simplification.