From: Justin Tobler <jltobler@gmail.com>
Date: Tue, 05 Mar 2024 22:30:18 GMT
Subject: Re: [PATCH 2/4] reftable/stack: register new tables as tempfiles
Message-ID: <6cw6d3ubo2kbogzdbniyoznij2zfoh5t3htwb4oaghaltcgeqg@kkrw4g6atr2k>
In-Reply-To: <02bf41d419efd00e510a89a405e1b046b166ba20.1709549619.git.ps@pks.im>

On 24/03/04 12:10PM, Patrick Steinhardt wrote:
> We do not register new tables which we're about to add to the stack with
> the tempfile API. Those tables will thus not be deleted in case Git gets
> killed.
> 
> Refactor the code to register tables as tempfiles.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  reftable/stack.c | 29 ++++++++++++-----------------
>  1 file changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/reftable/stack.c b/reftable/stack.c
> index b64e55648a..81544fbfa0 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -737,8 +737,9 @@ int reftable_addition_add(struct reftable_addition *add,
>  	struct strbuf tab_file_name = STRBUF_INIT;
>  	struct strbuf next_name = STRBUF_INIT;
>  	struct reftable_writer *wr = NULL;
> +	struct tempfile *tab_file = NULL;
>  	int err = 0;
> -	int tab_fd = 0;
> +	int tab_fd;
>  
>  	strbuf_reset(&next_name);
>  	format_name(&next_name, add->next_update_index, add->next_update_index);
> @@ -746,17 +747,20 @@ int reftable_addition_add(struct reftable_addition *add,
>  	stack_filename(&temp_tab_file_name, add->stack, next_name.buf);
>  	strbuf_addstr(&temp_tab_file_name, ".temp.XXXXXX");
>  
> -	tab_fd = mkstemp(temp_tab_file_name.buf);
> -	if (tab_fd < 0) {
> +	tab_file = mks_tempfile(temp_tab_file_name.buf);
> +	if (!tab_file) {
>  		err = REFTABLE_IO_ERROR;
>  		goto done;
>  	}
>  	if (add->stack->config.default_permissions) {
> -		if (chmod(temp_tab_file_name.buf, add->stack->config.default_permissions)) {
> +		if (chmod(get_tempfile_path(tab_file),
> +			  add->stack->config.default_permissions)) {
>  			err = REFTABLE_IO_ERROR;
>  			goto done;
>  		}
>  	}

Since the tempfile is now being created through the tempfile API, I
think the file mode can be set directly through `mks_tempfile_m()`
instead of creating the tempfile and then using chmod. Just something I
thought to mention.

> +	tab_fd = get_tempfile_fd(tab_file);

-Justin