Re: [PATCH 3/6] t-reftable-stack: use Git's tempfile API instead of mkstemp()

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

On Tue, Aug 06, 2024 at 07:43:39PM +0530, Chandra Pratap wrote:
> Git's tempfile API defined by $GIT_DIR/tempfile.{c, h} provides
> a unified interface for tempfile operations. Since reftable/stack.c
> uses this API for all its tempfile needs instead of raw functions
> like mkstemp(), make the ported stack test strictly use Git's
> tempfile API as well.
> 
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> ---
>  t/unit-tests/t-reftable-stack.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
> index e033feb8ee..14909b127e 100644
> --- a/t/unit-tests/t-reftable-stack.c
> +++ b/t/unit-tests/t-reftable-stack.c
> @@ -76,7 +76,8 @@ static char *get_tmp_dir(int linenumber)
>  static void t_read_file(void)
>  {
>  	char *fn = get_tmp_template(__LINE__);
> -	int fd = mkstemp(fn);
> +	struct tempfile *tmp = mks_tempfile(fn);
> +	int fd = get_tempfile_fd(tmp);
>  	char out[1024] = "line1\n\nline2\nline3";
>  	int n, err;
>  	char **names = NULL;
> @@ -95,6 +96,7 @@ static void t_read_file(void)
>  		check_str(want[i], names[i]);
>  	free_names(names);
>  	(void) remove(fn);
> +	delete_tempfile(&tmp);
>  }

I'm a bit torn whether this is a good idea because we are using a higher
level interface that doesn't have unit tests itself. As I see it, both
low-level primitives and anything that is already verified via another
set of unit tests can be used when writing unit tests. But as far as I
know, the tempfile interface does not yet have any.

Maybe I'm being overthinking this though.

Ideally, we wouldn't have to care about the underlying issue at all,
namely cleanup of the temporary file. This is something that the clar
brings to us for free: it can create temporary directories that it also
knows to clean up automatically once done.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]