Re: [PATCH v3] setup: don't die if realpath(3) fails on getcwd(3)

[Kevin Locke <kevin@kevinlocke.name>]

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 */
>> -	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