On 3/20/2019 10:04 PM, Junio C Hamano wrote:
Josh Steadmon <steadmon@xxxxxxxxxx> writes:
[...]
+/*
+ * How many attempts we will make at creating a random trace output path.
+ */
+#define MAX_RANDOM_ATTEMPTS 10
With the updated design, randomness is no longer the primary
property of this new feature. The fact that the names are
automatically assigned is. It could be that the source of tr2_sid
may (or may not) be some randomness, but the point is that the
caller in this patch does not care how tr2_sid is computed.
I'd call this max-attempts (or max-autopath-attempts, but that is
rather long, and I do not think inside the scope of "tr2_dst" that
is about "destination", there will be anything but the destination
path we'd "attempt" with a reasonable maximum value to compute, so
the "-autopath" clarification would not buy us much)....
static int tr2_dst_want_warning(void)
{
static int tr2env_dst_debug = -1;
@@ -36,6 +42,53 @@ void tr2_dst_trace_disable(struct tr2_dst *dst)
dst->need_close = 0;
}
+static int tr2_dst_try_random_path(struct tr2_dst *dst, const char *tgt_prefix)
.... and I'd call this s/random/auto/ instead, if I were writing
this patch following the updated design.
I agree with Junio WRT the s/random/auto/ suggestions.
[...]
+ 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.
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.
+ fd = open(final_path.buf, O_WRONLY | O_CREAT | O_EXCL, 0666);
[...]
Nice. Thanks for looking into this.
Jeff
