Re: [PATCH 1/4] wrapper: add git_mkdtemp()

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 5, 2025, 23:05 UTC
Message-ID: <xmqqikek7cn1.fsf@gitster.g>
In-Reply-To: <65c997a7-e480-4617-a761-fc9dc8a7b20d@web.de>

René Scharfe <l.s.r@web.de> writes:

> -int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
> +static int git_mkdstemps_mode(char *pattern, int suffix_len, int mode, bool dir)

This is a file-scope static, so as long as it is understood by those who futz with things in this file well, there is no need to go extra mile to avoid confusion, but the meaning of the returned value from this function is vastly different depending on the value of "dir". It used to be that you can subject it to write(2), but obviously that is not relevant when you called mkdir(2) here.

Show 10 quoted lines

>  {
>  	static const char letters[] =
>  		"abcdefghijklmnopqrstuvwxyz"
> @@ -471,7 +471,10 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
>  			v /= num_letters;
>  		}
>  
> -		fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
> +		if (dir)
> +			fd = mkdir(pattern, mode);

OK. The caller calls this helper with 0700 (S_IRWXU), so that's probably OK. If we can make this into two helper functions with distinct function signatures that share the majority of logic, it would have been much nicer, but short of introducing a callback function I do not think of a good way, so I'll let it pass.

> +		else
> +			fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
>  		if (fd >= 0)
>  			return fd;