Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
strbuf_grow(): maintain nul-termination even for new buffer
In the case where sb is initialized to the slopbuf (through
strbuf_init(sb,0) or STRBUF_INIT), strbuf_grow() loses the terminating
nul: it grows the buffer, but gives ALLOC_GROW a NULL source to avoid
it being freed.  So ALLOC_GROW does not copy anything to the new
memory area.

This subtly broke the call to strbuf_getline in read_next_command()
[fast-import.c:1855], which goes

    strbuf_detach(&command_buf, NULL);  # command_buf is now = STRBUF_INIT
    stdin_eof = strbuf_getline(&command_buf, stdin, '\n');
    if (stdin_eof)
            return EOF;

In strbuf_getwholeline, this did

    strbuf_grow(sb, 0);  # loses nul-termination
    if (feof(fp))
            return EOF;
    strbuf_reset(sb);    # this would have nul-terminated!

Valgrind found this because fast-import subsequently uses prefixcmp()
on command_buf.buf, which after the EOF exit contains only
uninitialized memory.

Arguably strbuf_getwholeline is also broken, in that it touches the
buffer before deciding whether to do any work.  However, it seems more
futureproof to not let the strbuf API lose the nul-termination by its
own fault.

So make sure that strbuf_grow() puts in a nul even if it has nowhere
to copy it from.  This makes strbuf_grow(sb, 0) a semantic no-op as
far as readers of the buffer are concerned.

Also remove the nul-termination added by strbuf_init, which is made
redudant.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
trast authored and gitster committed Aug 29, 2011
1 parent f696543 commit 8c74ef1
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions strbuf.c
Expand Up @@ -30,10 +30,8 @@ void strbuf_init(struct strbuf *sb, size_t hint)
{
sb->alloc = sb->len = 0;
sb->buf = strbuf_slopbuf;
if (hint) {
if (hint)
strbuf_grow(sb, hint);
sb->buf[0] = '\0';
}
}

void strbuf_release(struct strbuf *sb)
Expand Down Expand Up @@ -65,12 +63,15 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)

void strbuf_grow(struct strbuf *sb, size_t extra)
{
int new_buf = !sb->alloc;
if (unsigned_add_overflows(extra, 1) ||
unsigned_add_overflows(sb->len, extra + 1))
die("you want to use way too much memory");
if (!sb->alloc)
if (new_buf)
sb->buf = NULL;
ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
if (new_buf)
sb->buf[0] = '\0';
}

void strbuf_trim(struct strbuf *sb)
Expand Down

0 comments on commit 8c74ef1

Please sign in to comment.