Re: [PATCH v2 3/4] strbuf_setlen: don't write to strbuf_slopbuf

Martin Ågren [off-list ref] writes:

On 23 August 2017 at 19:24, Junio C Hamano [off-list ref] wrote:

quoted

Martin Ågren [off-list ref] writes:

quoted

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. :-)

quoted

quoted

Suggested-by: Junio C Hamano <redacted>
Signed-off-by: Martin Ågren <redacted>
---
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.