On Tue, 2022-05-24 at 13:41 -0400, Derrick Stolee wrote:

On 5/24/2022 10:51 AM, Kevin Locke wrote:> -	/* Normalize the directory */

quoted

-	strbuf_realpath(&tmp, tmp_original_cwd, 1);
-	free((char*)tmp_original_cwd);
-	tmp_original_cwd = NULL;
-	startup_info->original_cwd = strbuf_detach(&tmp, NULL);
+	/* Try to normalize the directory. */
+	if (strbuf_realpath(&tmp, tmp_original_cwd, 0)) {
+		free((char*)tmp_original_cwd);
+		tmp_original_cwd = NULL;
+		startup_info->original_cwd = strbuf_detach(&tmp, NULL);
+	} else {
+		trace2_data_string("setup", the_repository,
+				   "realpath-path", tmp_original_cwd);
+		trace2_data_string("setup", the_repository,
+				   "realpath-failure", strerror(errno));
+		tmp_original_cwd = NULL;

I didn't catch this in v2, but should this line instead be

		startup_info->original_cwd = NULL;

? Especially based on this comment:

No worries.  It's a good question.  Setting startup_info->original_cwd
to NULL is handled by the no_prevention_needed label.  But I just
realized it's not actually required in this case, since original_cwd
is NULL when setup_original_cwd() is called.  I should probably return
rather than jumping to no_prevention_needed from the else block to
avoid the unnecessary free(NULL) and assignment.

Your comment made me realize that v2 and later neglect to free
tmp_original_cwd when strbuf_realpath() fails.  How embarrassing.

I'll send an update to fix both those issues shortly.

Thanks again,
Kevin