Re: [PATCH v2 1/1] trace2: write to directory targets

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 22, 2019, 03:30 UTC
Message-ID: <xmqqpnqjr3g0.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <51e88650-8667-df1f-13ef-4537f2e70346@jeffhostetler.com>

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 12 quoted lines

>>> +	for (attempt_count = 0; attempt_count < MAX_RANDOM_ATTEMPTS; attempt_count++) {
>>> +		strbuf_reset(&final_path);
>>> +		strbuf_addbuf(&final_path, &base_path);
>>> +		strbuf_addf(&final_path, ".%d", attempt_count);
>
> Since the last component of the SID is already very unique and
> we're unlikely to have collisions, maybe change the above line to:
>
> 		if (attempt_count > 0)
> 			strbuf_addf(&final_path, ".%d", attempt_count);
>
> and in reality expect to never have files with the suffix.

That's a nice property.

> Unless, that is, they turned on more than one of GIT_TR2,
> GIT_TR2_PERF, or GIT_TR2_EVENT and pointed them at the same
> directory, but I'm not sure if I care about that edge case
> or not.

That actually makes me wonder if the auto generated filenames want to have a common trait (e.g. suffix) that allows the readers to tell from which of these environment variables the names came from. It would not be very useful if two files with the same sid component had .1 suffix for GIT_TR2 trace for one session, and the same .1 suffix is used for GIT_TR2_PERF trace for a pair of files from another session.

But let's not worry about it for now. If people do not want them get mixed up and become hard to tell apart, they can always specify different directories for different traces.

Show 6 quoted lines

>
>>> +		fd = open(final_path.buf, O_WRONLY | O_CREAT | O_EXCL, 0666);
> [...]
>
> Nice.  Thanks for looking into this.
> Jeff