On Tue, Oct 03, 2017 at 03:24:14PM -0700, Jonathan Nieder wrote: > Here's a patch to address the surprising strbuf.h advice. > > -- >8 -- > Subject: strbuf: do not encourage init-after-release > > strbuf_release already leaves the strbuf in a valid, initialized > state, so there is not need to call strbuf_init after it. > > Moreover, this is not likely to change in the future: strbuf_release > leaving the strbuf in a valid state has been easy to maintain and has > been very helpful for Git's robustness and simplicity (e.g., > preventing use-after-free vulnerabilities).
Thanks for picking this up. Like you, I was quite surprised when I saw Stefan's original patch. > diff --git a/strbuf.h b/strbuf.h > index 7496cb8ec5..6e175c3694 100644 > --- a/strbuf.h > +++ b/strbuf.h > @@ -83,7 +83,7 @@ extern void strbuf_init(struct strbuf *, size_t); > > /** > * Release a string buffer and the memory it used. You should not use the > - * string buffer after using this function, unless you initialize it again. > + * string buffer after using this function. > */ > extern void strbuf_release(struct strbuf *); I think it's actually OK to use the string buffer after this function. It's just an empty string. Perhaps we should be more explicit: this releases any resources and resets to a pristine, empty state. I suspect strbuf_detach() probably should make the same claim. Earlier you mentioned: > It is still not advisable to call strbuf_release until done using a > strbuf because it is wasteful, so keep that part of the advice. Is this what you meant? If so, I think we should probably be more explicit in giving people a hint to use strbuf_reset() for efficiency. -Peff
