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

[Junio C Hamano <gitster@pobox.com>]

Jeff Hostetler <git@jeffhostetler.com> writes:

>>> +	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.

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