On 23 August 2017 at 19:24, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Martin Ågren <martin.agren@xxxxxxxxx> writes: > >> strbuf_setlen(., 0) writes '\0' to sb.buf[0], where buf is either >> allocated and unique to sb, or the global slopbuf. The slopbuf is meant >> to provide a guarantee that buf is not NULL and that a freshly >> initialized buffer contains the empty string, but it is not supposed to >> be written to. That strbuf_setlen writes to slopbuf has at least two >> implications: >> >> First, it's wrong in principle. Second, it might be hiding misuses which >> are just waiting to wreak havoc. Third, ThreadSanitizer detects a race >> when multiple threads write to slopbuf at roughly the same time, thus >> potentially making any more critical races harder to spot. > > There are two hard things in computer science ;-). Indeed. :-) >> Suggested-by: Junio C Hamano <gitster@xxxxxxxxx> >> Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx> >> --- >> v2: no "ifdef TSAN"; moved check from strbuf_reset into strbuf_setlen > > Looks much better. I have a mild objection to "suggested-by", > though. It makes it sound as if this were my itch, but it is not. > > All the credit for being motivate to fix the issue should go to you. > For what I did during the review of the previous one to lead to this > simpler version, if you want to document it, "helped-by" would be > more appropriate. Ok, so that's two things to tweak in the commit message. I'll hold off on v3 in case I get some more feedback the coming days. Thanks. Martin
