Re: [PATCH 1/8] Introduce new static function real_path_internal()
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:53
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH 1/8] Introduce new static function real_path_internal() · Junio C Hamano <hidden>
- 2016-06-15 · [PATCH 1/8] Introduce new static function real_path_internal() · Michael Haggerty <hidden>
Michael Haggerty [off-list ref] writes:
quoted hunk
@@ -54,20 +73,36 @@ const char *real_path(const char *path) } if (*buf) { - if (!*cwd && !getcwd(cwd, sizeof(cwd))) - die_errno ("Could not get current working directory"); + if (!*cwd && !getcwd(cwd, sizeof(cwd))) { + if (die_on_error) + die_errno("Could not get current working directory"); + else + goto error_out; + } - if (chdir(buf)) - die_errno ("Could not switch to '%s'", buf); + if (chdir(buf)) { + if (die_on_error) + die_errno("Could not switch to '%s'", buf); + else + goto error_out; + } + }
The patch makes sense, but while you are touching this code, I have to wonder if there is an easy way to tell, before entering the loop, if we have to chdir() around in the loop. That would allow us to hoist the getcwd() that is done only so that we can come back to where we started outside the loop, making it clear why the call is there, instead of cryptic "if (!*cwd &&" to ensure we do getcwd() once and before doing any chdir().