Skip to content

Commit

Permalink
strbuf: always return a non-NULL value from strbuf_detach
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Oct 18, 2012
1 parent 785ee49 commit 08ad56f
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion strbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ void strbuf_release(struct strbuf *sb)

char *strbuf_detach(struct strbuf *sb, size_t *sz)
{
char *res = sb->alloc ? sb->buf : NULL;
char *res;
strbuf_grow(sb, 0);
res = sb->buf;
if (sz)
*sz = sb->len;
strbuf_init(sb, 0);
Expand Down

0 comments on commit 08ad56f

Please sign in to comment.