Merge branch 'ma/ts-cleanups' into maint

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
This commit is contained in:
Junio C Hamano 2017-10-23 14:19:02 +09:00
commit dd3bfe4f5f
6 changed files with 37 additions and 3 deletions

10
.tsan-suppressions Normal file
View File

@ -0,0 +1,10 @@
# Suppressions for ThreadSanitizer (tsan).
#
# This file is used by setting the environment variable TSAN_OPTIONS to, e.g.,
# "suppressions=$(pwd)/.tsan-suppressions". Observe that relative paths such as
# ".tsan-suppressions" might not work.
# A static variable is written to racily, but we always write the same value, so
# in practice it (hopefully!) doesn't matter.
race:^want_color$
race:^transfer_debug$

View File

@ -2170,7 +2170,10 @@ static void *threaded_find_deltas(void *arg)
{
struct thread_params *me = arg;
progress_lock();
while (me->remaining) {
progress_unlock();
find_deltas(me->list, &me->remaining,
me->window, me->depth, me->processed);
@ -2192,7 +2195,10 @@ static void *threaded_find_deltas(void *arg)
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;
}

View File

@ -338,6 +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)

View File

@ -1041,7 +1041,6 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
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) {
@ -1055,12 +1054,14 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
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)

View File

@ -154,7 +154,10 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len)
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]);
}
/**

View File

@ -1117,6 +1117,13 @@ int transport_helper_init(struct transport *transport, const char *name)
__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;