Re: [PATCH v14 03/16] quote.c: remove path_relative, use relative_path instead

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 24, 2013, 19:11 UTC
Message-ID: <7vhagn8gtz.fsf@alter.siamese.dyndns.org>
In-Reply-To: <654b2387c27470c3dc81169e8f43c231aa9504ce.1372087065.git.worldhello.net@gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

> Since there is an enhanced version of relative_path() in path.c,
> remove duplicate counterpart path_relative() in quote.c.

There is no nice comparison chart before and after like you had in patch 02/16?

Show 8 quoted lines

>  void write_name_quoted_relative(const char *name, size_t len,
>  				const char *prefix, size_t prefix_len,
>  				FILE *fp, int terminator)
>  {
>  	struct strbuf sb = STRBUF_INIT;
>  
> -	name = path_relative(name, len, &sb, prefix, prefix_len);
> +	name = relative_path(name, prefix, &sb);

Are we sure nobody calls prefix_len pointing into the middle of string, not at the end of "prefix"? This is unsafe for such a caller, and to make sure we catch them, we should remove the now-unused prefix_len parameter from this function.

Show 62 quoted lines

>  	write_name_quoted(name, fp, terminator);
>  
>  	strbuf_release(&sb);
>  }
>  
> -/*
> - * Give path as relative to prefix.
> - *
> - * The strbuf may or may not be used, so do not assume it contains the
> - * returned path.
> - */
> -static const char *path_relative(const char *in, int len,
> -				 struct strbuf *sb, const char *prefix,
> -				 int prefix_len)
> -{
> -	int off, i;
> -
> -	if (len < 0)
> -		len = strlen(in);
> -	if (prefix_len < 0) {
> -		if (prefix)
> -			prefix_len = strlen(prefix);
> -		else
> -			prefix_len = 0;
> -	}
> -
> -	off = 0;
> -	i = 0;
> -	while (i < prefix_len && i < len && prefix[i] == in[i]) {
> -		if (prefix[i] == '/')
> -			off = i + 1;
> -		i++;
> -	}
> -	in += off;
> -	len -= off;
> -
> -	if (i >= prefix_len)
> -		return in;
> -
> -	strbuf_reset(sb);
> -	strbuf_grow(sb, len);
> -
> -	while (i < prefix_len) {
> -		if (prefix[i] == '/')
> -			strbuf_addstr(sb, "../");
> -		i++;
> -	}
> -	strbuf_add(sb, in, len);
> -
> -	return sb->buf;
> -}
> -
>  /* quote path as relative to the given prefix */
>  char *quote_path_relative(const char *in, int len,
>  			  struct strbuf *out, const char *prefix)
>  {
>  	struct strbuf sb = STRBUF_INIT;
> -	const char *rel = path_relative(in, len, &sb, prefix, -1);
> +	const char *rel = relative_path(in, prefix, &sb);
>  	strbuf_reset(out);
>  	quote_c_style_counted(rel, strlen(rel), out, NULL, 0);
>  	strbuf_release(&sb);