Re: [PATCH 1/8] Introduce new static function real_path_internal()

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 27, 2012, 21:27 UTC
Message-ID: <7vk3vfp36g.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1348688090-13648-2-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 22 quoted lines

> @@ -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().