Re: [PATCH v15 03/16] quote.c: substitute path_relative with relative_path

[Junio C Hamano <gitster@pobox.com>]

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

> Substitute the function path_relative in quote.c with the function
> relative_path. Function relative_path can be treated as an enhanced
> and robust version of path_relative.
> ...
> And if prefix has no trailing slash, path_relative can not work properly
> either. But since prefix always has a trailing slash, so it's not a
> problem.

Nicely explained.

>  static void write_name(const char* name, size_t len)
>  {
> -	write_name_quoted_relative(name, len, prefix, prefix_len, stdout,
> -			line_terminator);
> +	/* Since prefix_len is ignored in write_name_quoted_relative, we
> +	 * should turn off prefix here in case of running "git ls-files"
> +	 * with "--full-name" option */
> +	write_name_quoted_relative(name, len, prefix_len ? prefix : NULL,
> +			prefix_len, stdout, line_terminator);
>  }

I'd tweak this function and write_naem_quoted_relative(), and
explain the reason for doing so in the log message.  Please check
what I'll push out on 'pu' later today.

-- >8 --
Subject: [PATCH] fix write_name() and write_name_quoted_relative() signature

The write_name_quoted_relative() function used to accept its two
parameters as counted strings.  Since it now uses relative_path(),
which requires both input strings to be NUL-terminated, the API of
this function need to be audited carefully.

Luckily, the only one caller is write_name() in builtin/ls-files.c,
and it in turn has only three callers.  They pass the string to be
made relative at this function and all of these strings happen to be
NUL terminated.  We can safely lose "len" parameter of write_name(),
and write_name_quoted_relative() can safely lose the length of its
name parameter as well.

The "prefix_len" parameter of write_name_quoted_relative() is either
0 (when "ls-files --full-name" is used), or the length of the prefix
string (i.e. the path to the current subdirectory).  By checking the
"--full-name" case in the caller, i.e. write_name(), we can make
write_name_quoted_relative() not to require the prefix as a counted
string.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/ls-files.c | 17 ++++++++---------
 quote.c            |  3 +--
 quote.h            |  5 ++---
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 247a8a4..d87e136 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -46,13 +46,12 @@ static const char *tag_modified = "";
 static const char *tag_skip_worktree = "";
 static const char *tag_resolve_undo = "";
 
-static void write_name(const char* name, size_t len)
+static void write_name(const char *name)
 {
-	/* Since prefix_len is ignored in write_name_quoted_relative, we
-	 * should turn off prefix here in case of running "git ls-files"
-	 * with "--full-name" option */
-	write_name_quoted_relative(name, len, prefix_len ? prefix : NULL,
-			prefix_len, stdout, line_terminator);
+	/* (prefix_len == 0) is for "--full-name" output */
+	write_name_quoted_relative(name,
+				   prefix_len ? prefix : NULL,
+				   stdout, line_terminator);
 }
 
 static void show_dir_entry(const char *tag, struct dir_entry *ent)
@@ -66,7 +65,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 		return;
 
 	fputs(tag, stdout);
-	write_name(ent->name, ent->len);
+	write_name(ent->name);
 }
 
 static void show_other_files(struct dir_struct *dir)
@@ -166,7 +165,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 		       find_unique_abbrev(ce->sha1,abbrev),
 		       ce_stage(ce));
 	}
-	write_name(ce->name, ce_namelen(ce));
+	write_name(ce->name);
 	if (debug_mode) {
 		printf("  ctime: %d:%d\n", ce->ce_ctime.sec, ce->ce_ctime.nsec);
 		printf("  mtime: %d:%d\n", ce->ce_mtime.sec, ce->ce_mtime.nsec);
@@ -199,7 +198,7 @@ static void show_ru_info(void)
 			printf("%s%06o %s %d\t", tag_resolve_undo, ui->mode[i],
 			       find_unique_abbrev(ui->sha1[i], abbrev),
 			       i + 1);
-			write_name(path, len);
+			write_name(path);
 		}
 	}
 }
diff --git a/quote.c b/quote.c
index 64ff344..6af3c83 100644
--- a/quote.c
+++ b/quote.c
@@ -312,8 +312,7 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen,
 	fputc(terminator, fp);
 }
 
-void write_name_quoted_relative(const char *name, size_t len,
-				const char *prefix, size_t prefix_len,
+void write_name_quoted_relative(const char *name, const char *prefix,
 				FILE *fp, int terminator)
 {
 	struct strbuf sb = STRBUF_INIT;
diff --git a/quote.h b/quote.h
index 133155a..8586bcd 100644
--- a/quote.h
+++ b/quote.h
@@ -60,9 +60,8 @@ extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);
 extern void write_name_quoted(const char *name, FILE *, int terminator);
 extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,
                                  const char *name, FILE *, int terminator);
-extern void write_name_quoted_relative(const char *name, size_t len,
-		const char *prefix, size_t prefix_len,
-		FILE *fp, int terminator);
+extern void write_name_quoted_relative(const char *name, const char *prefix,
+				       FILE *fp, int terminator);
 
 /* quote path as relative to the given prefix */
 extern char *quote_path_relative(const char *in, int len,
-- 
1.8.3.1-773-g7e1bb7d