On 09/27/2012 11:27 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
>
>> @@ -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().
I don't see an easy way to predict, before entering the loop, whether
chdir() will be needed. For example, compare
touch filename
ln -s filename foo
./test-path-utils real_path foo
with
touch filename
ln -s $(pwd)/filename foo
./test-path-utils real_path foo
In the first case no chdir() is needed, whereas in the second case a
chdir() is needed but only on the second loop iteration.
All I can offer you is a palliative like the one below.
Michael
diff --git a/abspath.c b/abspath.c
index 5748b91..40cdc46 100644
--- a/abspath.c
+++ b/abspath.c
@@ -35,7 +35,14 @@ static const char *real_path_internal(const char
*path, int die_on_error)
{
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf =
bufs[1];
char *retval = NULL;
+
+ /*
+ * If we have to temporarily chdir(), store the original CWD
+ * here so that we can chdir() back to it at the end of the
+ * function:
+ */
char cwd[1024] = "";
+
int buf_index = 1;
int depth = MAXDEPTH;
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html