strbuf change: be sure ->buf is never ever NULL.

For that purpose, the ->buf is always initialized with a char * buf living
in the strbuf module. It is made a char * so that we can sloppily accept
things that perform: sb->buf[0] = '\0', and because you can't pass "" as an
initializer for ->buf without making gcc unhappy for very good reasons.

strbuf_init/_detach/_grow have been fixed to trust ->alloc and not ->buf
anymore.

as a consequence strbuf_detach is _mandatory_ to detach a buffer, copying
->buf isn't an option anymore, if ->buf is going to escape from the scope,
and eventually be free'd.

API changes:
* strbuf_setlen now always works, so just make strbuf_reset a convenience
macro.
* strbuf_detatch takes a size_t* optional argument (meaning it can be
NULL) to copy the buffer's len, as it was needed for this refactor to
make the code more readable, and working like the callers.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-apply.c b/builtin-apply.c
index 01c9d60642d0faa9ea91d0d39b32985890e1ec7f..740623e6c09cbf85bd8d54aac560dc2c5015078b 100644 (file)
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -178,14 +178,13 @@ static void say_patch_name(FILE *output, const char *pre, struct patch *patch, c
 #define CHUNKSIZE (8192)
 #define SLOP (16)
 
 #define CHUNKSIZE (8192)
 #define SLOP (16)
 

-static void *read_patch_file(int fd, unsigned long *sizep)
+static void *read_patch_file(int fd, size_t *sizep)

 {
        struct strbuf buf;
 
        strbuf_init(&buf, 0);
        if (strbuf_read(&buf, fd, 0) < 0)
                die("git-apply: read returned %s", strerror(errno));
 {
        struct strbuf buf;
 
        strbuf_init(&buf, 0);
        if (strbuf_read(&buf, fd, 0) < 0)
                die("git-apply: read returned %s", strerror(errno));

-       *sizep = buf.len;

 
        /*
         * Make sure that we have some slop in the buffer
 
        /*
         * Make sure that we have some slop in the buffer


@@ -194,7 +193,7 @@ static void *read_patch_file(int fd, unsigned long *sizep)
         */
        strbuf_grow(&buf, SLOP);
        memset(buf.buf + buf.len, 0, SLOP);
         */
        strbuf_grow(&buf, SLOP);
        memset(buf.buf + buf.len, 0, SLOP);

-       return strbuf_detach(&buf);
+       return strbuf_detach(&buf, sizep);

 }
 
 static unsigned long linelen(const char *buffer, unsigned long size)
 }
 
 static unsigned long linelen(const char *buffer, unsigned long size)


@@ -253,7 +252,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)
                                 */
                                strbuf_remove(&name, 0, cp - name.buf);
                                free(def);
                                 */
                                strbuf_remove(&name, 0, cp - name.buf);
                                free(def);

-                               return name.buf;
+                               return strbuf_detach(&name, NULL);

                        }
                }
                strbuf_release(&name);
                        }
                }
                strbuf_release(&name);


@@ -607,7 +606,7 @@ static char *git_header_name(char *line, int llen)
                        if (strcmp(cp + 1, first.buf))
                                goto free_and_fail1;
                        strbuf_release(&sp);
                        if (strcmp(cp + 1, first.buf))
                                goto free_and_fail1;
                        strbuf_release(&sp);

-                       return first.buf;
+                       return strbuf_detach(&first, NULL);

                }
 
                /* unquoted second */
                }
 
                /* unquoted second */


@@ -618,7 +617,7 @@ static char *git_header_name(char *line, int llen)
                if (line + llen - cp != first.len + 1 ||
                    memcmp(first.buf, cp, first.len))
                        goto free_and_fail1;
                if (line + llen - cp != first.len + 1 ||
                    memcmp(first.buf, cp, first.len))
                        goto free_and_fail1;

-               return first.buf;
+               return strbuf_detach(&first, NULL);

 
        free_and_fail1:
                strbuf_release(&first);
 
        free_and_fail1:
                strbuf_release(&first);


@@ -655,7 +654,7 @@ static char *git_header_name(char *line, int llen)
                            isspace(name[len])) {
                                /* Good */
                                strbuf_remove(&sp, 0, np - sp.buf);
                            isspace(name[len])) {
                                /* Good */
                                strbuf_remove(&sp, 0, np - sp.buf);

-                               return sp.buf;
+                               return strbuf_detach(&sp, NULL);

                        }
 
                free_and_fail2:
                        }
 
                free_and_fail2:


@@ -1968,8 +1967,7 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *
 
        if (apply_fragments(&buf, patch) < 0)
                return -1; /* note with --reject this succeeds. */
 
        if (apply_fragments(&buf, patch) < 0)
                return -1; /* note with --reject this succeeds. */

-       patch->result = buf.buf;
-       patch->resultsize = buf.len;
+       patch->result = strbuf_detach(&buf, &patch->resultsize);

 
        if (0 < patch->is_delete && patch->resultsize)
                return error("removal patch leaves file contents");
 
        if (0 < patch->is_delete && patch->resultsize)
                return error("removal patch leaves file contents");

diff --git a/builtin-archive.c b/builtin-archive.c
index 843a9e37bbb5b7274e928499d3b7935399216e65..04385dea05110053db72e30a77e6d4a10bc7875b 100644 (file)
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -89,7 +89,7 @@ static void format_subst(const struct commit *commit,
        struct strbuf fmt;
 
        if (src == buf->buf)
        struct strbuf fmt;
 
        if (src == buf->buf)

-               to_free = strbuf_detach(buf);
+               to_free = strbuf_detach(buf, NULL);

        strbuf_init(&fmt, 0);
        for (;;) {
                const char *b, *c;
        strbuf_init(&fmt, 0);
        for (;;) {
                const char *b, *c;


@@ -153,8 +153,7 @@ void *sha1_file_to_archive(const char *path, const unsigned char *sha1,
                strbuf_attach(&buf, buffer, *sizep, *sizep + 1);
                convert_to_working_tree(path, buf.buf, buf.len, &buf);
                convert_to_archive(path, buf.buf, buf.len, &buf, commit);
                strbuf_attach(&buf, buffer, *sizep, *sizep + 1);
                convert_to_working_tree(path, buf.buf, buf.len, &buf);
                convert_to_archive(path, buf.buf, buf.len, &buf, commit);

-               *sizep = buf.len;
-               buffer = buf.buf;
+               buffer = strbuf_detach(&buf, sizep);

        }
 
        return buffer;
        }
 
        return buffer;

diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 349b59c25865fb8ea3946c4176cc2aa08869af1e..1e43d792216248c1abe3504c239ccd325f8d5ef1 100644 (file)
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -10,7 +10,7 @@ static char *get_stdin(void)
        if (strbuf_read(&buf, 0, 1024) < 0) {
                die("error reading standard input: %s", strerror(errno));
        }
        if (strbuf_read(&buf, 0, 1024) < 0) {
                die("error reading standard input: %s", strerror(errno));
        }

-       return strbuf_detach(&buf);
+       return strbuf_detach(&buf, NULL);

 }
 
 static void show_new(enum object_type type, unsigned char *sha1_new)
 }
 
 static void show_new(enum object_type type, unsigned char *sha1_new)

diff --git a/commit.c b/commit.c
index 55b08ec0b93040785792afbe0c4013fe410a9d40..62cc74d7a9a2e7cddd5048602e4c793269287add 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -663,7 +663,7 @@ static char *replace_encoding_header(char *buf, const char *encoding)
                                          len - strlen("encoding \n"),
                                          encoding, strlen(encoding));
        }
                                          len - strlen("encoding \n"),
                                          encoding, strlen(encoding));
        }

-       return tmp.buf;
+       return strbuf_detach(&tmp, NULL);

 }
 
 static char *logmsg_reencode(const struct commit *commit,
 }
 
 static char *logmsg_reencode(const struct commit *commit,

diff --git a/convert.c b/convert.c
index 79c9df2e918efe16da825895708c4ac6f08bfc62..0d5e909c696dac8b900713a992c72cd9cacf94e3 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -168,7 +168,7 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
 
        /* are we "faking" in place editing ? */
        if (src == buf->buf)
 
        /* are we "faking" in place editing ? */
        if (src == buf->buf)

-               to_free = strbuf_detach(buf);
+               to_free = strbuf_detach(buf, NULL);

 
        strbuf_grow(buf, len + stats.lf - stats.crlf);
        for (;;) {
 
        strbuf_grow(buf, len + stats.lf - stats.crlf);
        for (;;) {


@@ -464,7 +464,7 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,
 
        /* are we "faking" in place editing ? */
        if (src == buf->buf)
 
        /* are we "faking" in place editing ? */
        if (src == buf->buf)

-               to_free = strbuf_detach(buf);
+               to_free = strbuf_detach(buf, NULL);

        hash_sha1_file(src, len, "blob", sha1);
 
        strbuf_grow(buf, len + cnt * 43);
        hash_sha1_file(src, len, "blob", sha1);
 
        strbuf_grow(buf, len + cnt * 43);

diff --git a/diff.c b/diff.c
index fb6d077f06338271f16bfabb3e0c45f000de15ac..ab575191d11801f023655fe6cf718bfa5fa04c9b 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -197,7 +197,7 @@ static char *quote_two(const char *one, const char *two)
                strbuf_addstr(&res, one);
                strbuf_addstr(&res, two);
        }
                strbuf_addstr(&res, one);
                strbuf_addstr(&res, two);
        }

-       return res.buf;
+       return strbuf_detach(&res, NULL);

 }
 
 static const char *external_diff(void)
 }
 
 static const char *external_diff(void)


@@ -662,7 +662,7 @@ static char *pprint_rename(const char *a, const char *b)
                quote_c_style(a, &name, NULL, 0);
                strbuf_addstr(&name, " => ");
                quote_c_style(b, &name, NULL, 0);
                quote_c_style(a, &name, NULL, 0);
                strbuf_addstr(&name, " => ");
                quote_c_style(b, &name, NULL, 0);

-               return name.buf;
+               return strbuf_detach(&name, NULL);

        }
 
        /* Find common prefix */
        }
 
        /* Find common prefix */


@@ -710,7 +710,7 @@ static char *pprint_rename(const char *a, const char *b)
                strbuf_addch(&name, '}');
                strbuf_add(&name, a + len_a - sfx_length, sfx_length);
        }
                strbuf_addch(&name, '}');
                strbuf_add(&name, a + len_a - sfx_length, sfx_length);
        }

-       return name.buf;
+       return strbuf_detach(&name, NULL);

 }
 
 struct diffstat_t {
 }
 
 struct diffstat_t {


@@ -827,7 +827,7 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
                        strbuf_init(&buf, 0);
                        if (quote_c_style(file->name, &buf, NULL, 0)) {
                                free(file->name);
                        strbuf_init(&buf, 0);
                        if (quote_c_style(file->name, &buf, NULL, 0)) {
                                free(file->name);

-                               file->name = buf.buf;
+                               file->name = strbuf_detach(&buf, NULL);

                        } else {
                                strbuf_release(&buf);
                        }
                        } else {
                                strbuf_release(&buf);
                        }


@@ -1519,8 +1519,7 @@ static int populate_from_stdin(struct diff_filespec *s)
                                     strerror(errno));
 
        s->should_munmap = 0;
                                     strerror(errno));
 
        s->should_munmap = 0;

-       s->size = buf.len;
-       s->data = strbuf_detach(&buf);
+       s->data = strbuf_detach(&buf, &s->size);

        s->should_free = 1;
        return 0;
 }
        s->should_free = 1;
        return 0;
 }


@@ -1612,8 +1611,7 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
                if (convert_to_git(s->path, s->data, s->size, &buf)) {
                        munmap(s->data, s->size);
                        s->should_munmap = 0;
                if (convert_to_git(s->path, s->data, s->size, &buf)) {
                        munmap(s->data, s->size);
                        s->should_munmap = 0;

-                       s->data = buf.buf;
-                       s->size = buf.len;
+                       s->data = strbuf_detach(&buf, &s->size);

                        s->should_free = 1;
                }
        }
                        s->should_free = 1;
                }
        }

diff --git a/entry.c b/entry.c
index 4a8c73bfaee14c84866f1a20b7fcd8421d3812f1..98f5f6d4ecfc0dabae9a920bdf3beadbf20abcaf 100644 (file)
--- a/entry.c
+++ b/entry.c
@@ -120,8 +120,7 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
                strbuf_init(&buf, 0);
                if (convert_to_working_tree(ce->name, new, size, &buf)) {
                        free(new);
                strbuf_init(&buf, 0);
                if (convert_to_working_tree(ce->name, new, size, &buf)) {
                        free(new);

-                       new = buf.buf;
-                       size = buf.len;
+                       new = strbuf_detach(&buf, &size);

                }
 
                if (to_tempfile) {
                }
 
                if (to_tempfile) {

diff --git a/fast-import.c b/fast-import.c
index a870a44e3d1a266f856738ac1242158482dc59dc..e9c80be4cd3f2833dd227e3d2f3c69e2145935e2 100644 (file)
--- a/fast-import.c
+++ b/fast-import.c
@@ -1562,7 +1562,7 @@ static int read_next_command(void)
                } else {
                        struct recent_command *rc;
 
                } else {
                        struct recent_command *rc;
 

-                       strbuf_detach(&command_buf);
+                       strbuf_detach(&command_buf, NULL);

                        stdin_eof = strbuf_getline(&command_buf, stdin, '\n');
                        if (stdin_eof)
                                return EOF;
                        stdin_eof = strbuf_getline(&command_buf, stdin, '\n');
                        if (stdin_eof)
                                return EOF;

diff --git a/imap-send.c b/imap-send.c
index e95cdde062b0741fa8c73ddc87ada2fbfab8921f..a429a76a6385bb7d7935cfaddec9cfc8508c77e5 100644 (file)
--- a/imap-send.c
+++ b/imap-send.c
@@ -1180,7 +1180,7 @@ read_message( FILE *f, msg_data_t *msg )
        } while (!feof(f));
 
        msg->len  = buf.len;
        } while (!feof(f));
 
        msg->len  = buf.len;

-       msg->data = strbuf_detach(&buf);
+       msg->data = strbuf_detach(&buf, NULL);

        return msg->len;
 }
 
        return msg->len;
 }
 

diff --git a/quote.c b/quote.c
index 800fd88c9ae9979bf44c91fea54c092cdafdb33c..482be05b7af159b9b47095fedfbdfa3bda65c748 100644 (file)
--- a/quote.c
+++ b/quote.c
@@ -22,7 +22,7 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
        char *to_free = NULL;
 
        if (dst->buf == src)
        char *to_free = NULL;
 
        if (dst->buf == src)

-               to_free = strbuf_detach(dst);
+               to_free = strbuf_detach(dst, NULL);

 
        strbuf_addch(dst, '\'');
        while (*src) {
 
        strbuf_addch(dst, '\'');
        while (*src) {

diff --git a/sha1_file.c b/sha1_file.c
index 385c5d891af9b7c79dd93e0e6777b0975ff5dbef..753742a47cad1b27dc0d52ce7097cf106e4a40be 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2340,8 +2340,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,
                strbuf_init(&nbuf, 0);
                if (convert_to_git(path, buf, size, &nbuf)) {
                        munmap(buf, size);
                strbuf_init(&nbuf, 0);
                if (convert_to_git(path, buf, size, &nbuf)) {
                        munmap(buf, size);

-                       size = nbuf.len;
-                       buf = nbuf.buf;
+                       buf = strbuf_detach(&nbuf, &size);

                        re_allocated = 1;
                }
        }
                        re_allocated = 1;
                }
        }

diff --git a/strbuf.c b/strbuf.c
index d1e338bfb695322cf2c90fa39bc8d482a3ade297..0e431daa6158108130b34c0b3a61f1875c70b023 100644 (file)
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,27 +1,33 @@
 #include "cache.h"
 
 #include "cache.h"
 

+/*
+ * Used as the default ->buf value, so that people can always assume
+ * buf is non NULL and ->buf is NUL terminated even for a freshly
+ * initialized strbuf.
+ */
+char strbuf_slopbuf[1];
+

 void strbuf_init(struct strbuf *sb, size_t hint)
 {
 void strbuf_init(struct strbuf *sb, size_t hint)
 {

-       memset(sb, 0, sizeof(*sb));
+       sb->alloc = sb->len = 0;
+       sb->buf = strbuf_slopbuf;

        if (hint)
                strbuf_grow(sb, hint);
 }
 
 void strbuf_release(struct strbuf *sb)
 {
        if (hint)
                strbuf_grow(sb, hint);
 }
 
 void strbuf_release(struct strbuf *sb)
 {

-       free(sb->buf);
-       memset(sb, 0, sizeof(*sb));
-}
-
-void strbuf_reset(struct strbuf *sb)
-{
-       if (sb->len)
-               strbuf_setlen(sb, 0);
+       if (sb->alloc) {
+               free(sb->buf);
+               strbuf_init(sb, 0);
+       }

 }
 
 }
 

-char *strbuf_detach(struct strbuf *sb)
+char *strbuf_detach(struct strbuf *sb, size_t *sz)

 {
 {

-       char *res = sb->buf;
+       char *res = sb->alloc ? sb->buf : NULL;
+       if (sz)
+               *sz = sb->len;

        strbuf_init(sb, 0);
        return res;
 }
        strbuf_init(sb, 0);
        return res;
 }


@@ -40,6 +46,8 @@ void strbuf_grow(struct strbuf *sb, size_t extra)
 {
        if (sb->len + extra + 1 <= sb->len)
                die("you want to use way too much memory");
 {
        if (sb->len + extra + 1 <= sb->len)
                die("you want to use way too much memory");

+       if (!sb->alloc)
+               sb->buf = NULL;

        ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
 }
 
        ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
 }
 

diff --git a/strbuf.h b/strbuf.h
index 5657e3db1401dda1eea7d8be2bd36ec36502b187..6deec78479ec9bf4c7dec0bcd4030534b887d92a 100644 (file)
--- a/strbuf.h
+++ b/strbuf.h
@@ -10,8 +10,7 @@
  * 1. the ->buf member is always malloc-ed, hence strbuf's can be used to
  *    build complex strings/buffers whose final size isn't easily known.
  *
  * 1. the ->buf member is always malloc-ed, hence strbuf's can be used to
  *    build complex strings/buffers whose final size isn't easily known.
  *

- *    It is legal to copy the ->buf pointer away. Though if you want to reuse
- *    the strbuf after that, setting ->buf to NULL isn't legal.
+ *    It is NOT legal to copy the ->buf pointer away.

  *    `strbuf_detach' is the operation that detachs a buffer from its shell
  *    while keeping the shell valid wrt its invariants.
  *
  *    `strbuf_detach' is the operation that detachs a buffer from its shell
  *    while keeping the shell valid wrt its invariants.
  *


@@ -41,19 +40,19 @@
 
 #include <assert.h>
 
 
 #include <assert.h>
 

+extern char strbuf_slopbuf[];

 struct strbuf {
        size_t alloc;
        size_t len;
        char *buf;
 };
 
 struct strbuf {
        size_t alloc;
        size_t len;
        char *buf;
 };
 

-#define STRBUF_INIT  { 0, 0, NULL }
+#define STRBUF_INIT  { 0, 0, strbuf_slopbuf }

 
 /*----- strbuf life cycle -----*/
 extern void strbuf_init(struct strbuf *, size_t);
 extern void strbuf_release(struct strbuf *);
 
 /*----- strbuf life cycle -----*/
 extern void strbuf_init(struct strbuf *, size_t);
 extern void strbuf_release(struct strbuf *);

-extern void strbuf_reset(struct strbuf *);
-extern char *strbuf_detach(struct strbuf *);
+extern char *strbuf_detach(struct strbuf *, size_t *);

 extern void strbuf_attach(struct strbuf *, void *, size_t, size_t);
 static inline void strbuf_swap(struct strbuf *a, struct strbuf *b) {
        struct strbuf tmp = *a;
 extern void strbuf_attach(struct strbuf *, void *, size_t, size_t);
 static inline void strbuf_swap(struct strbuf *a, struct strbuf *b) {
        struct strbuf tmp = *a;


@@ -75,6 +74,7 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
        sb->len = len;
        sb->buf[len] = '\0';
 }
        sb->len = len;
        sb->buf[len] = '\0';
 }

+#define strbuf_reset(sb)  strbuf_setlen(sb, 0)

 
 /*----- content related -----*/
 extern void strbuf_rtrim(struct strbuf *);
 
 /*----- content related -----*/
 extern void strbuf_rtrim(struct strbuf *);