Re: [PATCH 6/7 v2] Compute prefix at runtime if RUNTIME_PREFIX is set

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 19, 2009, 17:41 UTC
Message-ID: <alpine.DEB.1.00.0901191825040.3586@pacific.mpi-cbg.de>
In-Reply-To: <1232280015-8144-7-git-send-email-prohaska@zib.de>

Hi,

On Sun, 18 Jan 2009, Steffen Prohaska wrote:

Show 10 quoted lines

> +	if (!prefix) {
> +		const char *strip[] = {
> +			GIT_EXEC_PATH,
> +			BINDIR,
> +			0
> +		};
> +		const char **s;
> +
> +		for (s = strip; *s; s++) {
> +			const char *sargv = argv0_path + strlen(argv0_path);

This does not need to be recomputed all the time, right?

Show 17 quoted lines

> +			const char *ss = *s + strlen(*s);
> +			while (argv0_path < sargv && *s < ss
> +				&& (*sargv == *ss ||
> +				    (is_dir_sep(*sargv) && is_dir_sep(*ss)))) {
> +				sargv--;
> +				ss--;
> +			}
> +			if (*s == ss) {
> +				struct strbuf d = STRBUF_INIT;
> +				/* We also skip the trailing directory separator. */
> +				assert(sargv - argv0_path - 1 >= 0);
> +				strbuf_add(&d, argv0_path, sargv - argv0_path - 1);
> +				prefix = strbuf_detach(&d, NULL);
> +				break;
> +			}
> +		}
> +	}

I have a definite feeling that this code would be easier to read if it resembled this:

/* sets prefix if the suffix matches (modulo separator changes) */
static int strip_path_suffix(const char *path, const char *suffix,
	char **prefix)
{
	int path_len = strlen(path), suffix_len = strlen(suffix);

	while (suffix_len) {
		if (!path_len)
			return 1;

		path_len--; suffix_len--;
		/* strip arbitrary amount of directory separators */
		if (is_dir_sep(path[path_len]) && 
				is_dir_sep(suffix[suffix_len])) {
			while (path_len && is_dir_sep(path[path_len]))
				path_len--;
			while (suffix_len && is_dir_sep(suffix[suffix_len]))
				suffix_len--;
		}
		else if (path[path_len] != suffix[suffix_len])
			return 1;
	}

	while (path_len && is_dir_sep(path[path_len - 1]))
		path_len--;

	prefix = xstrndup(path, path_len);
	return 0;
}

...

	if (!prefix && strip_path_suffix(argv0_path, GIT_EXEC_PATH) &&
			strip_path_suffix(argv0_path, BINDIR)) {
		prefix = PREFIX;
		...

Note: this function could be exported in path.c, as it also handles the 
"a//bin" vs "a/bin" case.

Ciao, Dscho