Martin Ågren <martin.agren@xxxxxxxxx> writes: > 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. Well, this one is good enough and your "at least two" is technically fine ;-) Let's not reroll this any further.
