Git fork

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

+37 -3
+10
.tsan-suppressions
··· 1 + # Suppressions for ThreadSanitizer (tsan). 2 + # 3 + # This file is used by setting the environment variable TSAN_OPTIONS to, e.g., 4 + # "suppressions=$(pwd)/.tsan-suppressions". Observe that relative paths such as 5 + # ".tsan-suppressions" might not work. 6 + 7 + # A static variable is written to racily, but we always write the same value, so 8 + # in practice it (hopefully!) doesn't matter. 9 + race:^want_color$ 10 + race:^transfer_debug$
+6
builtin/pack-objects.c
··· 2171 2171 { 2172 2172 struct thread_params *me = arg; 2173 2173 2174 + progress_lock(); 2174 2175 while (me->remaining) { 2176 + progress_unlock(); 2177 + 2175 2178 find_deltas(me->list, &me->remaining, 2176 2179 me->window, me->depth, me->processed); 2177 2180 ··· 2193 2196 pthread_cond_wait(&me->cond, &me->mutex); 2194 2197 me->data_ready = 0; 2195 2198 pthread_mutex_unlock(&me->mutex); 2199 + 2200 + progress_lock(); 2196 2201 } 2202 + progress_unlock(); 2197 2203 /* leave ->working 1 so that this doesn't get more work assigned */ 2198 2204 return NULL; 2199 2205 }
+7
color.c
··· 338 338 339 339 int want_color(int var) 340 340 { 341 + /* 342 + * NEEDSWORK: This function is sometimes used from multiple threads, and 343 + * we end up using want_auto racily. That "should not matter" since 344 + * we always write the same value, but it's still wrong. This function 345 + * is listed in .tsan-suppressions for the time being. 346 + */ 347 + 341 348 static int want_auto = -1; 342 349 343 350 if (var < 0)
+3 -2
convert.c
··· 1040 1040 ca->crlf_action = git_path_check_crlf(ccheck + 4); 1041 1041 if (ca->crlf_action == CRLF_UNDEFINED) 1042 1042 ca->crlf_action = git_path_check_crlf(ccheck + 0); 1043 - ca->attr_action = ca->crlf_action; 1044 1043 ca->ident = git_path_check_ident(ccheck + 1); 1045 1044 ca->drv = git_path_check_convert(ccheck + 2); 1046 1045 if (ca->crlf_action != CRLF_BINARY) { ··· 1054 1053 else if (eol_attr == EOL_CRLF) 1055 1054 ca->crlf_action = CRLF_TEXT_CRLF; 1056 1055 } 1057 - ca->attr_action = ca->crlf_action; 1058 1056 } else { 1059 1057 ca->drv = NULL; 1060 1058 ca->crlf_action = CRLF_UNDEFINED; 1061 1059 ca->ident = 0; 1062 1060 } 1061 + 1062 + /* Save attr and make a decision for action */ 1063 + ca->attr_action = ca->crlf_action; 1063 1064 if (ca->crlf_action == CRLF_TEXT) 1064 1065 ca->crlf_action = text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT; 1065 1066 if (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_FALSE)
+4 -1
strbuf.h
··· 147 147 if (len > (sb->alloc ? sb->alloc - 1 : 0)) 148 148 die("BUG: strbuf_setlen() beyond buffer"); 149 149 sb->len = len; 150 - sb->buf[len] = '\0'; 150 + if (sb->buf != strbuf_slopbuf) 151 + sb->buf[len] = '\0'; 152 + else 153 + assert(!strbuf_slopbuf[0]); 151 154 } 152 155 153 156 /**
+7
transport-helper.c
··· 1117 1117 __attribute__((format (printf, 1, 2))) 1118 1118 static void transfer_debug(const char *fmt, ...) 1119 1119 { 1120 + /* 1121 + * NEEDSWORK: This function is sometimes used from multiple threads, and 1122 + * we end up using debug_enabled racily. That "should not matter" since 1123 + * we always write the same value, but it's still wrong. This function 1124 + * is listed in .tsan-suppressions for the time being. 1125 + */ 1126 + 1120 1127 va_list args; 1121 1128 char msgbuf[PBUFFERSIZE]; 1122 1129 static int debug_enabled = -1;