strbuf: always return a non-NULL value from strbuf_detach

The current behavior is to return NULL when strbuf did not actually allocate a string. This can be quite surprising to callers, though, who may feed the strbuf from arbitrary data and expect to always get a valid value. In most cases, it does not make a difference because calling any strbuf function will cause an allocation (even if the function ends up not inserting any data). But if the code is structured like: struct strbuf buf = STRBUF_INIT; if (some_condition) strbuf_addstr(&buf, some_string); return strbuf_detach(&buf, NULL); then you may or may not return NULL, depending on the condition. This can cause us to segfault in http-push (when fed an empty URL) and in http-backend (when an empty parameter like "foo=bar&&" is in the $QUERY_STRING). This patch forces strbuf_detach to allocate an empty NUL-terminated string when it is called on a strbuf that has not been allocated. I investigated all call-sites of strbuf_detach. The majority are either not affected by the change (because they call a strbuf_* function unconditionally), or can handle the empty string just as easily as NULL. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 18, 2012 at 06:00 UTC 08ad56f3f0c1df8f75bac84bcef0d9d0c9b4d20f
1 file changed +3 -1
strbuf.c
+3 -1
@@ -44,7 +44,9 @@ void strbuf_release(struct strbuf *sb)
44
45 char *strbuf_detach(struct strbuf *sb, size_t *sz)
46 {
47 - char *res = sb->alloc ? sb->buf : NULL;
47 + char *res;
48 + strbuf_grow(sb, 0);
49 + res = sb->buf;
50 if (sz)
51 *sz = sb->len;
52 strbuf_init(sb, 0);