From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 10 Sep 2017 08:08:22 +0000 (+0900)
Subject: Merge branch 'ma/ts-cleanups'
X-Git-Tag: v2.15.0-rc0~108
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/a48ce378585054f2ae3b2f0e18346ab005290524?ds=inline;hp=-c

Merge branch 'ma/ts-cleanups'

Assorted bugfixes and clean-ups.

* ma/ts-cleanups:
ThreadSanitizer: add suppressions
strbuf_setlen: don't write to strbuf_slopbuf
pack-objects: take lock before accessing `remaining`
convert: always initialize attr_action in convert_attrs
---

a48ce378585054f2ae3b2f0e18346ab005290524
diff --combined builtin/pack-objects.c
index 82ad6e0c81,ffb13f7800..a57b4f058d
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@@ -25,7 -25,6 +25,7 @@@
  #include "sha1-array.h"
  #include "argv-array.h"
  #include "mru.h"
 +#include "packfile.h"
  
  static const char *pack_usage[] = {
  	N_("git pack-objects --stdout [<options>...] [< <ref-list> | < <object-list>]"),
@@@ -1290,7 -1289,7 +1290,7 @@@ static int done_pbase_path_pos(unsigne
  
  static int check_pbase_path(unsigned hash)
  {
 -	int pos = (!done_pbase_paths) ? -1 : done_pbase_path_pos(hash);
 +	int pos = done_pbase_path_pos(hash);
  	if (0 <= pos)
  		return 1;
  	pos = -pos - 1;
@@@ -1299,8 -1298,9 +1299,8 @@@
  		   done_pbase_paths_alloc);
  	done_pbase_paths_num++;
  	if (pos < done_pbase_paths_num)
 -		memmove(done_pbase_paths + pos + 1,
 -			done_pbase_paths + pos,
 -			(done_pbase_paths_num - pos - 1) * sizeof(unsigned));
 +		MOVE_ARRAY(done_pbase_paths + pos + 1, done_pbase_paths + pos,
 +			   done_pbase_paths_num - pos - 1);
  	done_pbase_paths[pos] = hash;
  	return 0;
  }
@@@ -2171,7 -2171,10 +2171,10 @@@ static void *threaded_find_deltas(void 
  {
  	struct thread_params *me = arg;
  
+ 	progress_lock();
  	while (me->remaining) {
+ 		progress_unlock();
+ 
  		find_deltas(me->list, &me->remaining,
  			    me->window, me->depth, me->processed);
  
@@@ -2193,7 -2196,10 +2196,10 @@@
  			pthread_cond_wait(&me->cond, &me->mutex);
  		me->data_ready = 0;
  		pthread_mutex_unlock(&me->mutex);
+ 
+ 		progress_lock();
  	}
+ 	progress_unlock();
  	/* leave ->working 1 so that this doesn't get more work assigned */
  	return NULL;
  }
diff --combined color.c
index 7aa8b076f0,9a9261ac16..9ccd954d6b
--- a/color.c
+++ b/color.c
@@@ -338,6 -338,13 +338,13 @@@ static int check_auto_color(void
  
  int want_color(int var)
  {
+ 	/*
+ 	 * NEEDSWORK: This function is sometimes used from multiple threads, and
+ 	 * we end up using want_auto racily. That "should not matter" since
+ 	 * we always write the same value, but it's still wrong. This function
+ 	 * is listed in .tsan-suppressions for the time being.
+ 	 */
+ 
  	static int want_auto = -1;
  
  	if (var < 0)
@@@ -361,6 -368,14 +368,6 @@@ int git_color_config(const char *var, c
  	return 0;
  }
  
 -int git_color_default_config(const char *var, const char *value, void *cb)
 -{
 -	if (git_color_config(var, value, cb) < 0)
 -		return -1;
 -
 -	return git_default_config(var, value, cb);
 -}
 -
  void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb)
  {
  	if (*color)
diff --combined convert.c
index c5f0b21037,77b7615a54..02962261c5
--- a/convert.c
+++ b/convert.c
@@@ -501,7 -501,6 +501,7 @@@ static int apply_single_file_filter(con
  
  #define CAP_CLEAN    (1u<<0)
  #define CAP_SMUDGE   (1u<<1)
 +#define CAP_DELAY    (1u<<2)
  
  struct cmd2process {
  	struct subprocess_entry subprocess; /* must be the first member! */
@@@ -513,49 -512,69 +513,49 @@@ static struct hashmap subprocess_map
  
  static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
  {
 -	int err;
 +	static int versions[] = {2, 0};
 +	static struct subprocess_capability capabilities[] = {
 +		{ "clean",  CAP_CLEAN  },
 +		{ "smudge", CAP_SMUDGE },
 +		{ "delay",  CAP_DELAY  },
 +		{ NULL, 0 }
 +	};
  	struct cmd2process *entry = (struct cmd2process *)subprocess;
 -	struct string_list cap_list = STRING_LIST_INIT_NODUP;
 -	char *cap_buf;
 -	const char *cap_name;
 -	struct child_process *process = &subprocess->process;
 -	const char *cmd = subprocess->cmd;
 -
 -	sigchain_push(SIGPIPE, SIG_IGN);
 -
 -	err = packet_writel(process->in, "git-filter-client", "version=2", NULL);
 -	if (err)
 -		goto done;
 -
 -	err = strcmp(packet_read_line(process->out, NULL), "git-filter-server");
 -	if (err) {
 -		error("external filter '%s' does not support filter protocol version 2", cmd);
 -		goto done;
 -	}
 -	err = strcmp(packet_read_line(process->out, NULL), "version=2");
 -	if (err)
 -		goto done;
 -	err = packet_read_line(process->out, NULL) != NULL;
 -	if (err)
 -		goto done;
 -
 -	err = packet_writel(process->in, "capability=clean", "capability=smudge", NULL);
 -
 -	for (;;) {
 -		cap_buf = packet_read_line(process->out, NULL);
 -		if (!cap_buf)
 -			break;
 -		string_list_split_in_place(&cap_list, cap_buf, '=', 1);
 -
 -		if (cap_list.nr != 2 || strcmp(cap_list.items[0].string, "capability"))
 -			continue;
 -
 -		cap_name = cap_list.items[1].string;
 -		if (!strcmp(cap_name, "clean")) {
 -			entry->supported_capabilities |= CAP_CLEAN;
 -		} else if (!strcmp(cap_name, "smudge")) {
 -			entry->supported_capabilities |= CAP_SMUDGE;
 -		} else {
 -			warning(
 -				"external filter '%s' requested unsupported filter capability '%s'",
 -				cmd, cap_name
 -			);
 -		}
 +	return subprocess_handshake(subprocess, "git-filter", versions, NULL,
 +				    capabilities,
 +				    &entry->supported_capabilities);
 +}
  
 -		string_list_clear(&cap_list, 0);
 +static void handle_filter_error(const struct strbuf *filter_status,
 +				struct cmd2process *entry,
 +				const unsigned int wanted_capability) {
 +	if (!strcmp(filter_status->buf, "error"))
 +		; /* The filter signaled a problem with the file. */
 +	else if (!strcmp(filter_status->buf, "abort") && wanted_capability) {
 +		/*
 +		 * The filter signaled a permanent problem. Don't try to filter
 +		 * files with the same command for the lifetime of the current
 +		 * Git process.
 +		 */
 +		 entry->supported_capabilities &= ~wanted_capability;
 +	} else {
 +		/*
 +		 * Something went wrong with the protocol filter.
 +		 * Force shutdown and restart if another blob requires filtering.
 +		 */
 +		error("external filter '%s' failed", entry->subprocess.cmd);
 +		subprocess_stop(&subprocess_map, &entry->subprocess);
 +		free(entry);
  	}
 -
 -done:
 -	sigchain_pop(SIGPIPE);
 -
 -	return err;
  }
  
  static int apply_multi_file_filter(const char *path, const char *src, size_t len,
  				   int fd, struct strbuf *dst, const char *cmd,
 -				   const unsigned int wanted_capability)
 +				   const unsigned int wanted_capability,
 +				   struct delayed_checkout *dco)
  {
  	int err;
 +	int can_delay = 0;
  	struct cmd2process *entry;
  	struct child_process *process;
  	struct strbuf nbuf = STRBUF_INIT;
@@@ -564,7 -583,8 +564,7 @@@
  
  	if (!subprocess_map_initialized) {
  		subprocess_map_initialized = 1;
 -		hashmap_init(&subprocess_map, (hashmap_cmp_fn) cmd2process_cmp,
 -			     NULL, 0);
 +		hashmap_init(&subprocess_map, cmd2process_cmp, NULL, 0);
  		entry = NULL;
  	} else {
  		entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd);
@@@ -583,12 -603,12 +583,12 @@@
  	}
  	process = &entry->subprocess.process;
  
 -	if (!(wanted_capability & entry->supported_capabilities))
 +	if (!(entry->supported_capabilities & wanted_capability))
  		return 0;
  
 -	if (CAP_CLEAN & wanted_capability)
 +	if (wanted_capability & CAP_CLEAN)
  		filter_type = "clean";
 -	else if (CAP_SMUDGE & wanted_capability)
 +	else if (wanted_capability & CAP_SMUDGE)
  		filter_type = "smudge";
  	else
  		die("unexpected filter type");
@@@ -610,14 -630,6 +610,14 @@@
  	if (err)
  		goto done;
  
 +	if ((entry->supported_capabilities & CAP_DELAY) &&
 +	    dco && dco->state == CE_CAN_DELAY) {
 +		can_delay = 1;
 +		err = packet_write_fmt_gently(process->in, "can-delay=1\n");
 +		if (err)
 +			goto done;
 +	}
 +
  	err = packet_flush_gently(process->in);
  	if (err)
  		goto done;
@@@ -633,73 -645,14 +633,73 @@@
  	if (err)
  		goto done;
  
 -	err = strcmp(filter_status.buf, "success");
 +	if (can_delay && !strcmp(filter_status.buf, "delayed")) {
 +		string_list_insert(&dco->filters, cmd);
 +		string_list_insert(&dco->paths, path);
 +	} else {
 +		/* The filter got the blob and wants to send us a response. */
 +		err = strcmp(filter_status.buf, "success");
 +		if (err)
 +			goto done;
 +
 +		err = read_packetized_to_strbuf(process->out, &nbuf) < 0;
 +		if (err)
 +			goto done;
 +
 +		err = subprocess_read_status(process->out, &filter_status);
 +		if (err)
 +			goto done;
 +
 +		err = strcmp(filter_status.buf, "success");
 +	}
 +
 +done:
 +	sigchain_pop(SIGPIPE);
 +
 +	if (err)
 +		handle_filter_error(&filter_status, entry, wanted_capability);
 +	else
 +		strbuf_swap(dst, &nbuf);
 +	strbuf_release(&nbuf);
 +	return !err;
 +}
 +
 +
 +int async_query_available_blobs(const char *cmd, struct string_list *available_paths)
 +{
 +	int err;
 +	char *line;
 +	struct cmd2process *entry;
 +	struct child_process *process;
 +	struct strbuf filter_status = STRBUF_INIT;
 +
 +	assert(subprocess_map_initialized);
 +	entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd);
 +	if (!entry) {
 +		error("external filter '%s' is not available anymore although "
 +		      "not all paths have been filtered", cmd);
 +		return 0;
 +	}
 +	process = &entry->subprocess.process;
 +	sigchain_push(SIGPIPE, SIG_IGN);
 +
 +	err = packet_write_fmt_gently(
 +		process->in, "command=list_available_blobs\n");
  	if (err)
  		goto done;
  
 -	err = read_packetized_to_strbuf(process->out, &nbuf) < 0;
 +	err = packet_flush_gently(process->in);
  	if (err)
  		goto done;
  
 +	while ((line = packet_read_line(process->out, NULL))) {
 +		const char *path;
 +		if (skip_prefix(line, "pathname=", &path))
 +			string_list_insert(available_paths, xstrdup(path));
 +		else
 +			; /* ignore unknown keys */
 +	}
 +
  	err = subprocess_read_status(process->out, &filter_status);
  	if (err)
  		goto done;
@@@ -709,8 -662,29 +709,8 @@@
  done:
  	sigchain_pop(SIGPIPE);
  
 -	if (err) {
 -		if (!strcmp(filter_status.buf, "error")) {
 -			/* The filter signaled a problem with the file. */
 -		} else if (!strcmp(filter_status.buf, "abort")) {
 -			/*
 -			 * The filter signaled a permanent problem. Don't try to filter
 -			 * files with the same command for the lifetime of the current
 -			 * Git process.
 -			 */
 -			 entry->supported_capabilities &= ~wanted_capability;
 -		} else {
 -			/*
 -			 * Something went wrong with the protocol filter.
 -			 * Force shutdown and restart if another blob requires filtering.
 -			 */
 -			error("external filter '%s' failed", cmd);
 -			subprocess_stop(&subprocess_map, &entry->subprocess);
 -			free(entry);
 -		}
 -	} else {
 -		strbuf_swap(dst, &nbuf);
 -	}
 -	strbuf_release(&nbuf);
 +	if (err)
 +		handle_filter_error(&filter_status, entry, 0);
  	return !err;
  }
  
@@@ -725,8 -699,7 +725,8 @@@ static struct convert_driver 
  
  static int apply_filter(const char *path, const char *src, size_t len,
  			int fd, struct strbuf *dst, struct convert_driver *drv,
 -			const unsigned int wanted_capability)
 +			const unsigned int wanted_capability,
 +			struct delayed_checkout *dco)
  {
  	const char *cmd = NULL;
  
@@@ -736,16 -709,15 +736,16 @@@
  	if (!dst)
  		return 1;
  
 -	if ((CAP_CLEAN & wanted_capability) && !drv->process && drv->clean)
 +	if ((wanted_capability & CAP_CLEAN) && !drv->process && drv->clean)
  		cmd = drv->clean;
 -	else if ((CAP_SMUDGE & wanted_capability) && !drv->process && drv->smudge)
 +	else if ((wanted_capability & CAP_SMUDGE) && !drv->process && drv->smudge)
  		cmd = drv->smudge;
  
  	if (cmd && *cmd)
  		return apply_single_file_filter(path, src, len, fd, dst, cmd);
  	else if (drv->process && *drv->process)
 -		return apply_multi_file_filter(path, src, len, fd, dst, drv->process, wanted_capability);
 +		return apply_multi_file_filter(path, src, len, fd, dst,
 +			drv->process, wanted_capability, dco);
  
  	return 0;
  }
@@@ -1040,7 -1012,6 +1040,6 @@@ static void convert_attrs(struct conv_a
  		ca->crlf_action = git_path_check_crlf(ccheck + 4);
  		if (ca->crlf_action == CRLF_UNDEFINED)
  			ca->crlf_action = git_path_check_crlf(ccheck + 0);
- 		ca->attr_action = ca->crlf_action;
  		ca->ident = git_path_check_ident(ccheck + 1);
  		ca->drv = git_path_check_convert(ccheck + 2);
  		if (ca->crlf_action != CRLF_BINARY) {
@@@ -1054,12 -1025,14 +1053,14 @@@
  			else if (eol_attr == EOL_CRLF)
  				ca->crlf_action = CRLF_TEXT_CRLF;
  		}
- 		ca->attr_action = ca->crlf_action;
  	} else {
  		ca->drv = NULL;
  		ca->crlf_action = CRLF_UNDEFINED;
  		ca->ident = 0;
  	}
+ 
+ 	/* Save attr and make a decision for action */
+ 	ca->attr_action = ca->crlf_action;
  	if (ca->crlf_action == CRLF_TEXT)
  		ca->crlf_action = text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;
  	if (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_FALSE)
@@@ -1086,7 -1059,7 +1087,7 @@@ int would_convert_to_git_filter_fd(cons
  	if (!ca.drv->required)
  		return 0;
  
 -	return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN);
 +	return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN, NULL);
  }
  
  const char *get_convert_attr_ascii(const char *path)
@@@ -1124,7 -1097,7 +1125,7 @@@ int convert_to_git(const struct index_s
  
  	convert_attrs(&ca, path);
  
 -	ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN);
 +	ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN, NULL);
  	if (!ret && ca.drv && ca.drv->required)
  		die("%s: clean filter '%s' failed", path, ca.drv->name);
  
@@@ -1132,12 -1105,10 +1133,12 @@@
  		src = dst->buf;
  		len = dst->len;
  	}
 -	ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);
 -	if (ret && dst) {
 -		src = dst->buf;
 -		len = dst->len;
 +	if (checksafe != SAFE_CRLF_KEEP_CRLF) {
 +		ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);
 +		if (ret && dst) {
 +			src = dst->buf;
 +			len = dst->len;
 +		}
  	}
  	return ret | ident_to_git(path, src, len, dst, ca.ident);
  }
@@@ -1152,7 -1123,7 +1153,7 @@@ void convert_to_git_filter_fd(const str
  	assert(ca.drv);
  	assert(ca.drv->clean || ca.drv->process);
  
 -	if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
 +	if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL))
  		die("%s: clean filter '%s' failed", path, ca.drv->name);
  
  	crlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
@@@ -1161,7 -1132,7 +1162,7 @@@
  
  static int convert_to_working_tree_internal(const char *path, const char *src,
  					    size_t len, struct strbuf *dst,
 -					    int normalizing)
 +					    int normalizing, struct delayed_checkout *dco)
  {
  	int ret = 0, ret_filter = 0;
  	struct conv_attrs ca;
@@@ -1186,30 -1157,22 +1187,30 @@@
  		}
  	}
  
 -	ret_filter = apply_filter(path, src, len, -1, dst, ca.drv, CAP_SMUDGE);
 +	ret_filter = apply_filter(
 +		path, src, len, -1, dst, ca.drv, CAP_SMUDGE, dco);
  	if (!ret_filter && ca.drv && ca.drv->required)
  		die("%s: smudge filter %s failed", path, ca.drv->name);
  
  	return ret | ret_filter;
  }
  
 +int async_convert_to_working_tree(const char *path, const char *src,
 +				  size_t len, struct strbuf *dst,
 +				  void *dco)
 +{
 +	return convert_to_working_tree_internal(path, src, len, dst, 0, dco);
 +}
 +
  int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)
  {
 -	return convert_to_working_tree_internal(path, src, len, dst, 0);
 +	return convert_to_working_tree_internal(path, src, len, dst, 0, NULL);
  }
  
  int renormalize_buffer(const struct index_state *istate, const char *path,
  		       const char *src, size_t len, struct strbuf *dst)
  {
 -	int ret = convert_to_working_tree_internal(path, src, len, dst, 1);
 +	int ret = convert_to_working_tree_internal(path, src, len, dst, 1, NULL);
  	if (ret) {
  		src = dst->buf;
  		len = dst->len;
diff --combined strbuf.h
index e705b94db5,1a77fe146a..7496cb8ec5
--- a/strbuf.h
+++ b/strbuf.h
@@@ -68,7 -68,7 +68,7 @@@ struct strbuf 
  };
  
  extern char strbuf_slopbuf[];
 -#define STRBUF_INIT  { 0, 0, strbuf_slopbuf }
 +#define STRBUF_INIT  { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }
  
  /**
   * Life Cycle Functions
@@@ -147,7 -147,10 +147,10 @@@ static inline void strbuf_setlen(struc
  	if (len > (sb->alloc ? sb->alloc - 1 : 0))
  		die("BUG: strbuf_setlen() beyond buffer");
  	sb->len = len;
- 	sb->buf[len] = '\0';
+ 	if (sb->buf != strbuf_slopbuf)
+ 		sb->buf[len] = '\0';
+ 	else
+ 		assert(!strbuf_slopbuf[0]);
  }
  
  /**
diff --combined transport-helper.c
index 8f68d69a86,2b830b2290..f50b34df2d
--- a/transport-helper.c
+++ b/transport-helper.c
@@@ -927,7 -927,7 +927,7 @@@ static int push_refs_with_export(struc
  		struct object_id oid;
  
  		private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);
 -		if (private && !get_sha1(private, oid.hash)) {
 +		if (private && !get_oid(private, &oid)) {
  			strbuf_addf(&buf, "^%s", private);
  			string_list_append(&revlist_args, strbuf_detach(&buf, NULL));
  			oidcpy(&ref->old_oid, &oid);
@@@ -1117,6 -1117,13 +1117,13 @@@ int transport_helper_init(struct transp
  __attribute__((format (printf, 1, 2)))
  static void transfer_debug(const char *fmt, ...)
  {
+ 	/*
+ 	 * NEEDSWORK: This function is sometimes used from multiple threads, and
+ 	 * we end up using debug_enabled racily. That "should not matter" since
+ 	 * we always write the same value, but it's still wrong. This function
+ 	 * is listed in .tsan-suppressions for the time being.
+ 	 */
+ 
  	va_list args;
  	char msgbuf[PBUFFERSIZE];
  	static int debug_enabled = -1;