* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Johannes Schindelin @ 2007-09-06 12:08 UTC (permalink / raw)
To: Miles Bader; +Cc: Dmitry Kakurin, Matthieu Moy, Git
In-Reply-To: <buoir6oo05v.fsf@dhapc248.dev.necel.com>
Hi,
On Thu, 6 Sep 2007, Miles Bader wrote:
> Dmitry Kakurin <dmitry.kakurin@gmail.com> writes:
> > When I first looked at Git source code two things struck me as odd:
> > 1. Pure C as opposed to C++. No idea why. Please don't talk about
> > portability, it's BS.
>
> Just to piss you off.
Hehe.
FWIW I strongly disagree that it's BS. As others have stated, the reasons
are easily found, and they are no weak arguments.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Invoke "git gc --auto" from "git add" and "git fetch"
From: Johannes Schindelin @ 2007-09-06 12:02 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nicolas Pitre, Nix, Steven Grimm, Linus Torvalds,
Git Mailing List
In-Reply-To: <7vhcm8j1bp.fsf_-_@gitster.siamese.dyndns.org>
Hi,
On Wed, 5 Sep 2007, Junio C Hamano wrote:
> * This is obviously a follow-up to the previous one that allows
> you to say "git gc --auto". I somewhat feel dirty about
> calling cmd_gc() bypassing fork & exec from "git add",
> though...
Since all git-gc seems to do is to fork() and exec() other git programs,
this should be fine (have not looked at cmd_gc() in a while, though).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git.__remotes_from_dir() should only return lists
From: Karl Hasselström @ 2007-09-06 11:26 UTC (permalink / raw)
To: Pavel Roskin; +Cc: Catalin Marinas, git
In-Reply-To: <20070905165722.17744.56584.stgit@dv.roinet.com>
On 2007-09-05 12:57:22 -0400, Pavel Roskin wrote:
> If there are no remotes, return empty list, not None. The later
> doesn't work with builtin set().
Thanks. But I guess an even nicer fix would be to make this function
return a set in the first place.
> This fixes t1001-branch-rename.sh
Hmm. I don't believe I saw t1001 break without this patch (I run the
test suite before I push, but I might have made a mistake of course).
Does the user's environment leak into the test sandbox?
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* [PATCH 5/7] Further strbuf re-engineering.
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11890776111670-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-apply.c | 30 +++++++-----------------
builtin-blame.c | 35 +++++++++------------------
builtin-commit-tree.c | 60 +++++++++++-------------------------------------
diff.c | 27 +++++++--------------
4 files changed, 44 insertions(+), 108 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 25b1447..d70c6cf 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -12,6 +12,7 @@
#include "blob.h"
#include "delta.h"
#include "builtin.h"
+#include "strbuf.h"
/*
* --check turns on checking that the working tree matches the
@@ -181,34 +182,21 @@ static void say_patch_name(FILE *output, const char *pre, struct patch *patch, c
static void *read_patch_file(int fd, unsigned long *sizep)
{
- unsigned long size = 0, alloc = CHUNKSIZE;
- void *buffer = xmalloc(alloc);
+ struct strbuf buf;
- for (;;) {
- ssize_t nr = alloc - size;
- if (nr < 1024) {
- alloc += CHUNKSIZE;
- buffer = xrealloc(buffer, alloc);
- nr = alloc - size;
- }
- nr = xread(fd, (char *) buffer + size, nr);
- if (!nr)
- break;
- if (nr < 0)
- die("git-apply: read returned %s", strerror(errno));
- size += nr;
- }
- *sizep = size;
+ strbuf_init(&buf);
+ if (strbuf_read(&buf, fd) < 0)
+ die("git-apply: read returned %s", strerror(errno));
+ *sizep = buf.len;
/*
* Make sure that we have some slop in the buffer
* so that we can do speculative "memcmp" etc, and
* see to it that it is NUL-filled.
*/
- if (alloc < size + SLOP)
- buffer = xrealloc(buffer, size + SLOP);
- memset((char *) buffer + size, 0, SLOP);
- return buffer;
+ strbuf_grow(&buf, SLOP);
+ memset(buf.buf + buf.len, 0, SLOP);
+ return strbuf_detach(&buf);
}
static unsigned long linelen(const char *buffer, unsigned long size)
diff --git a/builtin-blame.c b/builtin-blame.c
index dc88a95..1b1e6da 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -18,6 +18,7 @@
#include "cache-tree.h"
#include "path-list.h"
#include "mailmap.h"
+#include "strbuf.h"
static char blame_usage[] =
"git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
@@ -2001,11 +2002,10 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
struct commit *commit;
struct origin *origin;
unsigned char head_sha1[20];
- char *buf;
+ struct strbuf buf;
const char *ident;
int fd;
time_t now;
- unsigned long fin_size;
int size, len;
struct cache_entry *ce;
unsigned mode;
@@ -2023,9 +2023,11 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
origin = make_origin(commit, path);
+ strbuf_init(&buf);
if (!contents_from || strcmp("-", contents_from)) {
struct stat st;
const char *read_from;
+ unsigned long fin_size;
if (contents_from) {
if (stat(contents_from, &st) < 0)
@@ -2038,19 +2040,19 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
read_from = path;
}
fin_size = xsize_t(st.st_size);
- buf = xmalloc(fin_size+1);
mode = canon_mode(st.st_mode);
switch (st.st_mode & S_IFMT) {
case S_IFREG:
fd = open(read_from, O_RDONLY);
if (fd < 0)
die("cannot open %s", read_from);
- if (read_in_full(fd, buf, fin_size) != fin_size)
+ if (strbuf_read(&buf, fd) != xsize_t(st.st_size))
die("cannot read %s", read_from);
break;
case S_IFLNK:
- if (readlink(read_from, buf, fin_size+1) != fin_size)
+ if (readlink(read_from, buf.buf, buf.alloc) != fin_size)
die("cannot readlink %s", read_from);
+ buf.len = fin_size;
break;
default:
die("unsupported file type %s", read_from);
@@ -2059,26 +2061,13 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
else {
/* Reading from stdin */
contents_from = "standard input";
- buf = NULL;
- fin_size = 0;
mode = 0;
- while (1) {
- ssize_t cnt = 8192;
- buf = xrealloc(buf, fin_size + cnt);
- cnt = xread(0, buf + fin_size, cnt);
- if (cnt < 0)
- die("read error %s from stdin",
- strerror(errno));
- if (!cnt)
- break;
- fin_size += cnt;
- }
- buf = xrealloc(buf, fin_size + 1);
+ if (strbuf_read(&buf, 0) < 0)
+ die("read error %s from stdin", strerror(errno));
}
- buf[fin_size] = 0;
- origin->file.ptr = buf;
- origin->file.size = fin_size;
- pretend_sha1_file(buf, fin_size, OBJ_BLOB, origin->blob_sha1);
+ origin->file.ptr = buf.buf;
+ origin->file.size = buf.len;
+ pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
commit->util = origin;
/*
diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index ccbcbe3..bc9502c 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -8,42 +8,13 @@
#include "tree.h"
#include "builtin.h"
#include "utf8.h"
+#include "strbuf.h"
#define BLOCKING (1ul << 14)
/*
* FIXME! Share the code with "write-tree.c"
*/
-static void init_buffer(char **bufp, unsigned int *sizep)
-{
- *bufp = xmalloc(BLOCKING);
- *sizep = 0;
-}
-
-static void add_buffer(char **bufp, unsigned int *sizep, const char *fmt, ...)
-{
- char one_line[2048];
- va_list args;
- int len;
- unsigned long alloc, size, newsize;
- char *buf;
-
- va_start(args, fmt);
- len = vsnprintf(one_line, sizeof(one_line), fmt, args);
- va_end(args);
- size = *sizep;
- newsize = size + len + 1;
- alloc = (size + 32767) & ~32767;
- buf = *bufp;
- if (newsize > alloc) {
- alloc = (newsize + 32767) & ~32767;
- buf = xrealloc(buf, alloc);
- *bufp = buf;
- }
- *sizep = newsize - 1;
- memcpy(buf + size, one_line, len);
-}
-
static void check_valid(unsigned char *sha1, enum object_type expect)
{
enum object_type type = sha1_object_info(sha1, NULL);
@@ -87,9 +58,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
int parents = 0;
unsigned char tree_sha1[20];
unsigned char commit_sha1[20];
- char comment[1000];
- char *buffer;
- unsigned int size;
+ struct strbuf buffer;
int encoding_is_utf8;
git_config(git_default_config);
@@ -118,8 +87,9 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
/* Not having i18n.commitencoding is the same as having utf-8 */
encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
- init_buffer(&buffer, &size);
- add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));
+ strbuf_init(&buffer);
+ strbuf_grow(&buffer, 8192); /* should avoid reallocs for the headers */
+ strbuf_addf(&buffer, "tree %s\n", sha1_to_hex(tree_sha1));
/*
* NOTE! This ordering means that the same exact tree merged with a
@@ -127,26 +97,24 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
* if everything else stays the same.
*/
for (i = 0; i < parents; i++)
- add_buffer(&buffer, &size, "parent %s\n", sha1_to_hex(parent_sha1[i]));
+ strbuf_addf(&buffer, "parent %s\n", sha1_to_hex(parent_sha1[i]));
/* Person/date information */
- add_buffer(&buffer, &size, "author %s\n", git_author_info(1));
- add_buffer(&buffer, &size, "committer %s\n", git_committer_info(1));
+ strbuf_addf(&buffer, "author %s\n", git_author_info(1));
+ strbuf_addf(&buffer, "committer %s\n", git_committer_info(1));
if (!encoding_is_utf8)
- add_buffer(&buffer, &size,
- "encoding %s\n", git_commit_encoding);
- add_buffer(&buffer, &size, "\n");
+ strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
+ strbuf_addch(&buffer, '\n');
/* And add the comment */
- while (fgets(comment, sizeof(comment), stdin) != NULL)
- add_buffer(&buffer, &size, "%s", comment);
+ if (strbuf_read(&buffer, 0) < 0)
+ die("git-commit-tree: read returned %s", strerror(errno));
/* And check the encoding */
- buffer[size] = '\0';
- if (encoding_is_utf8 && !is_utf8(buffer))
+ if (encoding_is_utf8 && !is_utf8(buffer.buf))
fprintf(stderr, commit_utf8_warn);
- if (!write_sha1_file(buffer, size, commit_type, commit_sha1)) {
+ if (!write_sha1_file(buffer.buf, buffer.len, commit_type, commit_sha1)) {
printf("%s\n", sha1_to_hex(commit_sha1));
return 0;
}
diff --git a/diff.c b/diff.c
index 0d30d05..c054b23 100644
--- a/diff.c
+++ b/diff.c
@@ -9,6 +9,7 @@
#include "xdiff-interface.h"
#include "color.h"
#include "attr.h"
+#include "strbuf.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -1545,26 +1546,16 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
static int populate_from_stdin(struct diff_filespec *s)
{
-#define INCREMENT 1024
- char *buf;
- unsigned long size;
- ssize_t got;
-
- size = 0;
- buf = NULL;
- while (1) {
- buf = xrealloc(buf, size + INCREMENT);
- got = xread(0, buf + size, INCREMENT);
- if (!got)
- break; /* EOF */
- if (got < 0)
- return error("error while reading from stdin %s",
+ struct strbuf buf;
+
+ strbuf_init(&buf);
+ if (strbuf_read(&buf, 0) < 0)
+ return error("error while reading from stdin %s",
strerror(errno));
- size += got;
- }
+
s->should_munmap = 0;
- s->data = buf;
- s->size = size;
+ s->size = buf.len;
+ s->data = strbuf_detach(&buf);
s->should_free = 1;
return 0;
}
--
1.5.3.1
^ permalink raw reply related
* [PATCH 7/7] More strbuf uses in cache-tree.c.
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11890776112641-git-send-email-madcoder@debian.org>
Should even be marginally faster.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
cache-tree.c | 59 +++++++++++++++++++++------------------------------------
1 files changed, 22 insertions(+), 37 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 077f034..76af6f5 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "strbuf.h"
#include "tree.h"
#include "cache-tree.h"
@@ -235,8 +236,7 @@ static int update_one(struct cache_tree *it,
int missing_ok,
int dryrun)
{
- unsigned long size, offset;
- char *buffer;
+ struct strbuf buffer;
int i;
if (0 <= it->entry_count && has_sha1_file(it->sha1))
@@ -293,9 +293,8 @@ static int update_one(struct cache_tree *it,
/*
* Then write out the tree object for this level.
*/
- size = 8192;
- buffer = xmalloc(size);
- offset = 0;
+ strbuf_init(&buffer);
+ strbuf_grow(&buffer, 8192);
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
@@ -332,15 +331,9 @@ static int update_one(struct cache_tree *it,
if (!ce->ce_mode)
continue; /* entry being removed */
- if (size < offset + entlen + 100) {
- size = alloc_nr(offset + entlen + 100);
- buffer = xrealloc(buffer, size);
- }
- offset += sprintf(buffer + offset,
- "%o %.*s", mode, entlen, path + baselen);
- buffer[offset++] = 0;
- hashcpy((unsigned char*)buffer + offset, sha1);
- offset += 20;
+ strbuf_grow(&buffer, entlen + 100);
+ strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
+ strbuf_add(&buffer, sha1, 20);
#if DEBUG
fprintf(stderr, "cache-tree update-one %o %.*s\n",
@@ -349,10 +342,10 @@ static int update_one(struct cache_tree *it,
}
if (dryrun)
- hash_sha1_file(buffer, offset, tree_type, it->sha1);
+ hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);
else
- write_sha1_file(buffer, offset, tree_type, it->sha1);
- free(buffer);
+ write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);
+ strbuf_release(&buffer);
it->entry_count = i;
#if DEBUG
fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
@@ -378,12 +371,10 @@ int cache_tree_update(struct cache_tree *it,
return 0;
}
-static void *write_one(struct cache_tree *it,
+static void write_one(struct cache_tree *it,
char *path,
int pathlen,
- char *buffer,
- unsigned long *size,
- unsigned long *offset)
+ struct strbuf *buffer)
{
int i;
@@ -393,13 +384,9 @@ static void *write_one(struct cache_tree *it,
* tree-sha1 (missing if invalid)
* subtree_nr "cache-tree" entries for subtrees.
*/
- if (*size < *offset + pathlen + 100) {
- *size = alloc_nr(*offset + pathlen + 100);
- buffer = xrealloc(buffer, *size);
- }
- *offset += sprintf(buffer + *offset, "%.*s%c%d %d\n",
- pathlen, path, 0,
- it->entry_count, it->subtree_nr);
+ strbuf_grow(buffer, pathlen + 100);
+ strbuf_add(buffer, path, pathlen);
+ strbuf_addf(buffer, "%c%d %d\n", 0, it->entry_count, it->subtree_nr);
#if DEBUG
if (0 <= it->entry_count)
@@ -412,8 +399,7 @@ static void *write_one(struct cache_tree *it,
#endif
if (0 <= it->entry_count) {
- hashcpy((unsigned char*)buffer + *offset, it->sha1);
- *offset += 20;
+ strbuf_add(buffer, it->sha1, 20);
}
for (i = 0; i < it->subtree_nr; i++) {
struct cache_tree_sub *down = it->down[i];
@@ -423,21 +409,20 @@ static void *write_one(struct cache_tree *it,
prev->name, prev->namelen) <= 0)
die("fatal - unsorted cache subtree");
}
- buffer = write_one(down->cache_tree, down->name, down->namelen,
- buffer, size, offset);
+ write_one(down->cache_tree, down->name, down->namelen, buffer);
}
- return buffer;
}
void *cache_tree_write(struct cache_tree *root, unsigned long *size_p)
{
char path[PATH_MAX];
- unsigned long size = 8192;
- char *buffer = xmalloc(size);
+ struct strbuf buffer;
- *size_p = 0;
path[0] = 0;
- return write_one(root, path, 0, buffer, &size, size_p);
+ strbuf_init(&buffer);
+ write_one(root, path, 0, &buffer);
+ *size_p = buffer.len;
+ return strbuf_detach(&buffer);
}
static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
--
1.5.3.1
^ permalink raw reply related
* [PATCH 6/7] Eradicate yet-another-buffer implementation in buitin-rerere.c
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11890776112309-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-rerere.c | 56 +++++++++++++++++------------------------------------
1 files changed, 18 insertions(+), 38 deletions(-)
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 29d057c..7ebf6f1 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -1,6 +1,7 @@
#include "builtin.h"
#include "cache.h"
#include "path-list.h"
+#include "strbuf.h"
#include "xdiff/xdiff.h"
#include "xdiff-interface.h"
@@ -66,41 +67,20 @@ static int write_rr(struct path_list *rr, int out_fd)
return commit_lock_file(&write_lock);
}
-struct buffer {
- char *ptr;
- int nr, alloc;
-};
-
-static void append_line(struct buffer *buffer, const char *line)
-{
- int len = strlen(line);
-
- if (buffer->nr + len > buffer->alloc) {
- buffer->alloc = alloc_nr(buffer->nr + len);
- buffer->ptr = xrealloc(buffer->ptr, buffer->alloc);
- }
- memcpy(buffer->ptr + buffer->nr, line, len);
- buffer->nr += len;
-}
-
-static void clear_buffer(struct buffer *buffer)
-{
- free(buffer->ptr);
- buffer->ptr = NULL;
- buffer->nr = buffer->alloc = 0;
-}
-
static int handle_file(const char *path,
unsigned char *sha1, const char *output)
{
SHA_CTX ctx;
char buf[1024];
int hunk = 0, hunk_no = 0;
- struct buffer minus = { NULL, 0, 0 }, plus = { NULL, 0, 0 };
- struct buffer *one = &minus, *two = +
+ struct strbuf minus, plus;
+ struct strbuf *one = &minus, *two = +
FILE *f = fopen(path, "r");
FILE *out;
+ strbuf_init(&minus);
+ strbuf_init(&plus);
+
if (!f)
return error("Could not open %s", path);
@@ -122,36 +102,36 @@ static int handle_file(const char *path,
else if (!prefixcmp(buf, "======="))
hunk = 2;
else if (!prefixcmp(buf, ">>>>>>> ")) {
- int one_is_longer = (one->nr > two->nr);
- int common_len = one_is_longer ? two->nr : one->nr;
- int cmp = memcmp(one->ptr, two->ptr, common_len);
+ int one_is_longer = (one->len > two->len);
+ int common_len = one_is_longer ? two->len : one->len;
+ int cmp = memcmp(one->buf, two->buf, common_len);
hunk_no++;
hunk = 0;
if ((cmp > 0) || ((cmp == 0) && one_is_longer)) {
- struct buffer *swap = one;
+ struct strbuf *swap = one;
one = two;
two = swap;
}
if (out) {
fputs("<<<<<<<\n", out);
- fwrite(one->ptr, one->nr, 1, out);
+ fwrite(one->buf, one->len, 1, out);
fputs("=======\n", out);
- fwrite(two->ptr, two->nr, 1, out);
+ fwrite(two->buf, two->len, 1, out);
fputs(">>>>>>>\n", out);
}
if (sha1) {
- SHA1_Update(&ctx, one->ptr, one->nr);
+ SHA1_Update(&ctx, one->buf, one->len);
SHA1_Update(&ctx, "\0", 1);
- SHA1_Update(&ctx, two->ptr, two->nr);
+ SHA1_Update(&ctx, two->buf, two->len);
SHA1_Update(&ctx, "\0", 1);
}
- clear_buffer(one);
- clear_buffer(two);
+ strbuf_release(one);
+ strbuf_release(two);
} else if (hunk == 1)
- append_line(one, buf);
+ strbuf_addstr(one, buf);
else if (hunk == 2)
- append_line(two, buf);
+ strbuf_addstr(two, buf);
else if (out)
fputs(buf, out);
}
--
1.5.3.1
^ permalink raw reply related
* [PATCH 1/7] Rework strbuf API and semantics.
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11890776114037-git-send-email-madcoder@debian.org>
The gory details are explained in strbuf.h. The change of semantics this
patch enforces is that the embeded buffer has always a '\0' character after
its last byte, to always make it a C-string. The offs-by-one changes are all
related to that very change.
A strbuf can be used to store byte arrays, or as an extended string
library. The `buf' member can be passed to any C legacy string function,
because strbuf operations always ensure there is a terminating \0 at the end
of the buffer, not accounted in the `len' field of the structure.
A strbuf can be used to generate a string/buffer whose final size is not
really known, and then "strbuf_detach" can be used to get the built buffer,
and keep the wrapping "strbuf" structure usable for further work again.
Other interesting feature: strbuf_grow(sb, size) ensure that there is
enough allocated space in `sb' to put `size' new octets of data in the
buffer. It helps avoiding reallocating data for nothing when the problem the
strbuf helps to solve has a known typical size.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
archive-tar.c | 2 +-
fast-import.c | 15 ++++----
mktree.c | 4 +--
strbuf.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++--------
strbuf.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 181 insertions(+), 28 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 66fe3e3..a0763c5 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -166,7 +166,7 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
sprintf(header.name, "%s.paxheader", sha1_to_hex(sha1));
} else {
if (verbose)
- fprintf(stderr, "%.*s\n", path->len, path->buf);
+ fprintf(stderr, "%.*s\n", (int)path->len, path->buf);
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
*header.typeflag = TYPEFLAG_DIR;
mode = (mode | 0777) & ~tar_umask;
diff --git a/fast-import.c b/fast-import.c
index 078079d..2f7baf4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1595,7 +1595,7 @@ static void read_next_command(void)
} else {
struct recent_command *rc;
- command_buf.buf = NULL;
+ strbuf_detach(&command_buf);
read_line(&command_buf, stdin, '\n');
if (command_buf.eof)
return;
@@ -1649,7 +1649,6 @@ static void *cmd_data (size_t *size)
size_t sz = 8192, term_len = command_buf.len - 5 - 2;
length = 0;
buffer = xmalloc(sz);
- command_buf.buf = NULL;
for (;;) {
read_line(&command_buf, stdin, '\n');
if (command_buf.eof)
@@ -1657,11 +1656,11 @@ static void *cmd_data (size_t *size)
if (term_len == command_buf.len
&& !strcmp(term, command_buf.buf))
break;
- ALLOC_GROW(buffer, length + command_buf.len, sz);
+ ALLOC_GROW(buffer, length + command_buf.len + 1, sz);
memcpy(buffer + length,
command_buf.buf,
- command_buf.len - 1);
- length += command_buf.len - 1;
+ command_buf.len);
+ length += command_buf.len;
buffer[length++] = '\n';
}
free(term);
@@ -2101,7 +2100,7 @@ static void cmd_new_commit(void)
}
/* file_change* */
- while (!command_buf.eof && command_buf.len > 1) {
+ while (!command_buf.eof && command_buf.len > 0) {
if (!prefixcmp(command_buf.buf, "M "))
file_change_m(b);
else if (!prefixcmp(command_buf.buf, "D "))
@@ -2256,7 +2255,7 @@ static void cmd_reset_branch(void)
else
b = new_branch(sp);
read_next_command();
- if (!cmd_from(b) && command_buf.len > 1)
+ if (!cmd_from(b) && command_buf.len > 0)
unread_command_buf = 1;
}
@@ -2273,7 +2272,7 @@ static void cmd_checkpoint(void)
static void cmd_progress(void)
{
- fwrite(command_buf.buf, 1, command_buf.len - 1, stdout);
+ fwrite(command_buf.buf, 1, command_buf.len, stdout);
fputc('\n', stdout);
fflush(stdout);
skip_optional_lf();
diff --git a/mktree.c b/mktree.c
index d86dde8..86de5eb 100644
--- a/mktree.c
+++ b/mktree.c
@@ -92,7 +92,6 @@ int main(int ac, char **av)
strbuf_init(&sb);
while (1) {
- int len;
char *ptr, *ntr;
unsigned mode;
enum object_type type;
@@ -101,7 +100,6 @@ int main(int ac, char **av)
read_line(&sb, stdin, line_termination);
if (sb.eof)
break;
- len = sb.len;
ptr = sb.buf;
/* Input is non-recursive ls-tree output format
* mode SP type SP sha1 TAB name
@@ -111,7 +109,7 @@ int main(int ac, char **av)
die("input format error: %s", sb.buf);
ptr = ntr + 1; /* type */
ntr = strchr(ptr, ' ');
- if (!ntr || sb.buf + len <= ntr + 41 ||
+ if (!ntr || sb.buf + sb.len <= ntr + 40 ||
ntr[41] != '\t' ||
get_sha1_hex(ntr + 1, sha1))
die("input format error: %s", sb.buf);
diff --git a/strbuf.c b/strbuf.c
index e33d06b..acc7fc8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -2,40 +2,114 @@
#include "strbuf.h"
void strbuf_init(struct strbuf *sb) {
- sb->buf = NULL;
- sb->eof = sb->alloc = sb->len = 0;
+ memset(sb, 0, sizeof(*sb));
}
-static void strbuf_begin(struct strbuf *sb) {
+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);
+ sb->eof = 0;
+}
+
+char *strbuf_detach(struct strbuf *sb) {
+ char *res = sb->buf;
strbuf_init(sb);
+ return res;
+}
+
+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");
+ ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
}
-static void inline strbuf_add(struct strbuf *sb, int ch) {
- if (sb->alloc <= sb->len) {
- sb->alloc = sb->alloc * 3 / 2 + 16;
- sb->buf = xrealloc(sb->buf, sb->alloc);
+void strbuf_add(struct strbuf *sb, const void *data, size_t len) {
+ strbuf_grow(sb, len);
+ memcpy(sb->buf + sb->len, data, len);
+ strbuf_setlen(sb, sb->len + len);
+}
+
+void strbuf_addf(struct strbuf *sb, const char *fmt, ...) {
+ int len;
+ va_list ap;
+
+ va_start(ap, fmt);
+ len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+ va_end(ap);
+ if (len < 0) {
+ len = 0;
}
- sb->buf[sb->len++] = ch;
+ if (len >= strbuf_avail(sb)) {
+ strbuf_grow(sb, len);
+ va_start(ap, fmt);
+ len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+ va_end(ap);
+ if (len >= strbuf_avail(sb)) {
+ die("this should not happen, your snprintf is broken");
+ }
+ }
+ strbuf_setlen(sb, sb->len + len);
}
-static void strbuf_end(struct strbuf *sb) {
- strbuf_add(sb, 0);
+size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f) {
+ size_t res;
+
+ strbuf_grow(sb, size);
+ res = fread(sb->buf + sb->len, 1, size, f);
+ if (res > 0) {
+ strbuf_setlen(sb, sb->len + res);
+ }
+ return res;
+}
+
+ssize_t strbuf_read(struct strbuf *sb, int fd)
+{
+ size_t oldlen = sb->len;
+
+ for (;;) {
+ ssize_t cnt;
+
+ strbuf_grow(sb, 8192);
+ cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
+ if (cnt < 0) {
+ strbuf_setlen(sb, oldlen);
+ return -1;
+ }
+ if (!cnt)
+ break;
+ sb->len += cnt;
+ }
+
+ sb->buf[sb->len] = '\0';
+ return sb->len - oldlen;
}
void read_line(struct strbuf *sb, FILE *fp, int term) {
int ch;
- strbuf_begin(sb);
if (feof(fp)) {
+ strbuf_release(sb);
sb->eof = 1;
return;
}
+
+ strbuf_reset(sb);
while ((ch = fgetc(fp)) != EOF) {
if (ch == term)
break;
- strbuf_add(sb, ch);
+ strbuf_grow(sb, 1);
+ sb->buf[sb->len++] = ch;
}
- if (ch == EOF && sb->len == 0)
+ if (ch == EOF && sb->len == 0) {
+ strbuf_release(sb);
sb->eof = 1;
- strbuf_end(sb);
+ }
+
+ strbuf_grow(sb, 1);
+ sb->buf[sb->len] = '\0';
}
+
diff --git a/strbuf.h b/strbuf.h
index 74cc012..b40dc99 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -1,13 +1,95 @@
#ifndef STRBUF_H
#define STRBUF_H
+
+/*
+ * Strbuf's can be use in many ways: as a byte array, or to store arbitrary
+ * long, overflow safe strings.
+ *
+ * Strbufs has some invariants that are very important to keep in mind:
+ *
+ * 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.
+ * `strbuf_detach' is the operation that detachs a buffer from its shell
+ * while keeping the shell valid wrt its invariants.
+ *
+ * 2. the ->buf member is a byte array that has at least ->len + 1 bytes
+ * allocated. The extra byte is used to store a '\0', allowing the ->buf
+ * member to be a valid C-string. Every strbuf function ensure this
+ * invariant is preserved.
+ *
+ * Note that it is OK to "play" with the buffer directly if you work it
+ * that way:
+ *
+ * strbuf_grow(sb, SOME_SIZE);
+ * // ... here the memory areay starting at sb->buf, and of length
+ * // sb_avail(sb) is all yours, and you are sure that sb_avail(sb) is at
+ * // least SOME_SIZE
+ * strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
+ *
+ * Of course, SOME_OTHER_SIZE must be smaller or equal to sb_avail(sb).
+ *
+ * Doing so is safe, though if it has to be done in many places, adding the
+ * missing API to the strbuf module is the way to go.
+ *
+ * XXX: do _not_ assume that the area that is yours is of size ->alloc - 1
+ * even if it's true in the current implementation. Alloc is somehow a
+ * "private" member that should not be messed with.
+ */
+
+#include <assert.h>
+
struct strbuf {
- int alloc;
- int len;
+ size_t alloc;
+ size_t len;
int eof;
char *buf;
};
+#define STRBUF_INIT { 0, 0, 0, NULL }
+
+/*----- strbuf life cycle -----*/
extern void strbuf_init(struct strbuf *);
+extern void strbuf_release(struct strbuf *);
+extern void strbuf_reset(struct strbuf *);
+extern char *strbuf_detach(struct strbuf *);
+
+/*----- strbuf size related -----*/
+static inline size_t strbuf_avail(struct strbuf *sb) {
+ return sb->alloc ? sb->alloc - sb->len - 1 : 0;
+}
+static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
+ assert (len < sb->alloc);
+ sb->len = len;
+ sb->buf[len] = '\0';
+}
+
+extern void strbuf_grow(struct strbuf *, size_t);
+
+/*----- add data in your buffer -----*/
+static inline void strbuf_addch(struct strbuf *sb, int c) {
+ strbuf_grow(sb, 1);
+ sb->buf[sb->len++] = c;
+ sb->buf[sb->len] = '\0';
+}
+
+extern void strbuf_add(struct strbuf *, const void *, size_t);
+static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
+ strbuf_add(sb, s, strlen(s));
+}
+static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {
+ strbuf_add(sb, sb2->buf, sb2->len);
+}
+
+__attribute__((format(printf,2,3)))
+extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
+
+extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
+/* XXX: if read fails, any partial read is undone */
+extern ssize_t strbuf_read(struct strbuf *, int fd);
+
extern void read_line(struct strbuf *, FILE *, int);
#endif /* STRBUF_H */
--
1.5.3.1
^ permalink raw reply related
* [PATCH 3/7] Use proper strbuf API, and also simplify cmd_data code.
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11890776111843-git-send-email-madcoder@debian.org>
This patch features the use of strbuf_detach, and prevent the programmer
to mess with allocation directly. The code is as efficent as before, just
more concise and more straightforward.
---
fast-import.c | 30 +++++++++++++-----------------
1 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 2f7baf4..74ff0fd 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -340,7 +340,7 @@ static struct tag *last_tag;
/* Input stream parsing */
static whenspec_type whenspec = WHENSPEC_RAW;
-static struct strbuf command_buf;
+static struct strbuf command_buf = STRBUF_INIT;
static int unread_command_buf;
static struct recent_command cmd_hist = {&cmd_hist, &cmd_hist, NULL};
static struct recent_command *cmd_tail = &cmd_hist;
@@ -1638,17 +1638,16 @@ static void cmd_mark(void)
static void *cmd_data (size_t *size)
{
- size_t length;
- char *buffer;
+ struct strbuf buffer;
+ strbuf_init(&buffer);
if (prefixcmp(command_buf.buf, "data "))
die("Expected 'data n' command, found: %s", command_buf.buf);
if (!prefixcmp(command_buf.buf + 5, "<<")) {
char *term = xstrdup(command_buf.buf + 5 + 2);
- size_t sz = 8192, term_len = command_buf.len - 5 - 2;
- length = 0;
- buffer = xmalloc(sz);
+ size_t term_len = command_buf.len - 5 - 2;
+
for (;;) {
read_line(&command_buf, stdin, '\n');
if (command_buf.eof)
@@ -1656,21 +1655,18 @@ static void *cmd_data (size_t *size)
if (term_len == command_buf.len
&& !strcmp(term, command_buf.buf))
break;
- ALLOC_GROW(buffer, length + command_buf.len + 1, sz);
- memcpy(buffer + length,
- command_buf.buf,
- command_buf.len);
- length += command_buf.len;
- buffer[length++] = '\n';
+ strbuf_addbuf(&buffer, &command_buf);
+ strbuf_addch(&buffer, '\n');
}
free(term);
}
else {
- size_t n = 0;
+ size_t n = 0, length;
+
length = strtoul(command_buf.buf + 5, NULL, 10);
- buffer = xmalloc(length);
+
while (n < length) {
- size_t s = fread(buffer + n, 1, length - n, stdin);
+ size_t s = strbuf_fread(&buffer, length - n, stdin);
if (!s && feof(stdin))
die("EOF in data (%lu bytes remaining)",
(unsigned long)(length - n));
@@ -1679,8 +1675,8 @@ static void *cmd_data (size_t *size)
}
skip_optional_lf();
- *size = length;
- return buffer;
+ *size = buffer.len;
+ return strbuf_detach(&buffer);
}
static int validate_raw_date(const char *src, char *result, int maxlen)
--
1.5.3.1
^ permalink raw reply related
* [PATCH 2/7] Simplify strbuf uses in archive-tar.c using the proper functions.
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <118907761140-git-send-email-madcoder@debian.org>
This is just cleaner way to deal with strbufs, using its API rather than
reinventing it in the module (e.g. strbuf_append_string is just the plain
strbuf_addstr function, and it was used to perform what strbuf_addch does
anyways).
---
archive-tar.c | 65 ++++++++++++++-------------------------------------------
1 files changed, 16 insertions(+), 49 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index a0763c5..c84d7c0 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -78,19 +78,6 @@ static void write_trailer(void)
}
}
-static void strbuf_append_string(struct strbuf *sb, const char *s)
-{
- int slen = strlen(s);
- int total = sb->len + slen;
- if (total + 1 > sb->alloc) {
- sb->buf = xrealloc(sb->buf, total + 1);
- sb->alloc = total + 1;
- }
- memcpy(sb->buf + sb->len, s, slen);
- sb->len = total;
- sb->buf[total] = '\0';
-}
-
/*
* pax extended header records have the format "%u %s=%s\n". %u contains
* the size of the whole string (including the %u), the first %s is the
@@ -100,26 +87,17 @@ static void strbuf_append_string(struct strbuf *sb, const char *s)
static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
const char *value, unsigned int valuelen)
{
- char *p;
- int len, total, tmp;
+ int len, tmp;
/* "%u %s=%s\n" */
len = 1 + 1 + strlen(keyword) + 1 + valuelen + 1;
for (tmp = len; tmp > 9; tmp /= 10)
len++;
- total = sb->len + len;
- if (total > sb->alloc) {
- sb->buf = xrealloc(sb->buf, total);
- sb->alloc = total;
- }
-
- p = sb->buf;
- p += sprintf(p, "%u %s=", len, keyword);
- memcpy(p, value, valuelen);
- p += valuelen;
- *p = '\n';
- sb->len = total;
+ strbuf_grow(sb, len);
+ strbuf_addf(sb, "%u %s=", len, keyword);
+ strbuf_add(sb, value, valuelen);
+ strbuf_addch(sb, '\n');
}
static unsigned int ustar_header_chksum(const struct ustar_header *header)
@@ -153,8 +131,7 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
struct strbuf ext_header;
memset(&header, 0, sizeof(header));
- ext_header.buf = NULL;
- ext_header.len = ext_header.alloc = 0;
+ strbuf_init(&ext_header);
if (!sha1) {
*header.typeflag = TYPEFLAG_GLOBAL_HEADER;
@@ -225,8 +202,8 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
if (ext_header.len > 0) {
write_entry(sha1, NULL, 0, ext_header.buf, ext_header.len);
- free(ext_header.buf);
}
+ strbuf_release(&ext_header);
write_blocked(&header, sizeof(header));
if (S_ISREG(mode) && buffer && size > 0)
write_blocked(buffer, size);
@@ -235,11 +212,11 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
static void write_global_extended_header(const unsigned char *sha1)
{
struct strbuf ext_header;
- ext_header.buf = NULL;
- ext_header.len = ext_header.alloc = 0;
+
+ strbuf_init(&ext_header);
strbuf_append_ext_header(&ext_header, "comment", sha1_to_hex(sha1), 40);
write_entry(NULL, NULL, 0, ext_header.buf, ext_header.len);
- free(ext_header.buf);
+ strbuf_release(&ext_header);
}
static int git_tar_config(const char *var, const char *value)
@@ -260,28 +237,18 @@ static int write_tar_entry(const unsigned char *sha1,
const char *base, int baselen,
const char *filename, unsigned mode, int stage)
{
- static struct strbuf path;
+ static struct strbuf path = STRBUF_INIT;
int filenamelen = strlen(filename);
void *buffer;
enum object_type type;
unsigned long size;
- if (!path.alloc) {
- path.buf = xmalloc(PATH_MAX);
- path.alloc = PATH_MAX;
- path.len = path.eof = 0;
- }
- if (path.alloc < baselen + filenamelen + 1) {
- free(path.buf);
- path.buf = xmalloc(baselen + filenamelen + 1);
- path.alloc = baselen + filenamelen + 1;
- }
- memcpy(path.buf, base, baselen);
- memcpy(path.buf + baselen, filename, filenamelen);
- path.len = baselen + filenamelen;
- path.buf[path.len] = '\0';
+ strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
+ strbuf_reset(&path);
+ strbuf_add(&path, base, baselen);
+ strbuf_add(&path, filename, filenamelen);
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
- strbuf_append_string(&path, "/");
+ strbuf_addch(&path, '/');
buffer = NULL;
size = 0;
} else {
--
1.5.3.1
^ permalink raw reply related
* [PATCH 4/7] Simplify write_tree using strbuf's.
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit
In-Reply-To: <11890776112292-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
mktree.c | 23 ++++++++---------------
1 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/mktree.c b/mktree.c
index 86de5eb..2e84889 100644
--- a/mktree.c
+++ b/mktree.c
@@ -44,30 +44,23 @@ static int ent_compare(const void *a_, const void *b_)
static void write_tree(unsigned char *sha1)
{
- char *buffer;
- unsigned long size, offset;
+ struct strbuf buf;
+ size_t size;
int i;
qsort(entries, used, sizeof(*entries), ent_compare);
for (size = i = 0; i < used; i++)
size += 32 + entries[i]->len;
- buffer = xmalloc(size);
- offset = 0;
+ strbuf_init(&buf);
+ strbuf_grow(&buf, size);
for (i = 0; i < used; i++) {
struct treeent *ent = entries[i];
-
- if (offset + ent->len + 100 < size) {
- size = alloc_nr(offset + ent->len + 100);
- buffer = xrealloc(buffer, size);
- }
- offset += sprintf(buffer + offset, "%o ", ent->mode);
- offset += sprintf(buffer + offset, "%s", ent->name);
- buffer[offset++] = 0;
- hashcpy((unsigned char*)buffer + offset, ent->sha1);
- offset += 20;
+ strbuf_addf(&buf, "%o %s%c", ent->mode, ent->name, '\0');
+ strbuf_add(&buf, ent->sha1, 20);
}
- write_sha1_file(buffer, offset, tree_type, sha1);
+
+ write_sha1_file(buf.buf, buf.len, tree_type, sha1);
}
static const char mktree_usage[] = "git-mktree [-z]";
--
1.5.3.1
^ permalink raw reply related
* strbuf new API, take 2 for inclusion
From: Pierre Habouzit @ 2007-09-06 11:20 UTC (permalink / raw)
To: git
In-Reply-To: <20070902224213.GB431@artemis.corp>
Here is a new patchset (with no wrong patchs and numbering this time)
with Junio's remarks integrated (the va_copy stuff, better documentation
and patch comments and so on).
strbuf_read remains a "I can read all the content of the file or the
caller is supposed to die()" operation for now, as it's how it's used
right now, this can be changed easily in the future if needs for it
exists.
I've also stripped as many STRBUF_INIT uses as possible, some people
didn't liked it. I've kept its use for "static" strbufs where it's way
more convenient that a function call.
^ permalink raw reply
* Re: Git's database structure
From: Wincent Colaiuta @ 2007-09-06 11:03 UTC (permalink / raw)
To: Junio C Hamano
Cc: Andreas Ericsson, Jon Smirl, Julian Phillips, Theodore Tso,
Git Mailing List
In-Reply-To: <7vsl5sb1nd.fsf@gitster.siamese.dyndns.org>
El 6/9/2007, a las 11:09, Junio C Hamano escribió:
> Andreas Ericsson <ae@op5.se> writes:
>
>> Estimated daily uses of git-blame, world-wide: few
>> Estimated daily uses of git-{merge,diff}, worldwide: lots
>
> Which makes the author of git-blame weep X-<.
But the few times when you do use git-blame (apart from when you use
it out of sheer curiosity) it usually saves you backside (ie. when
you've located a problem in the code and you want to know the who/
what/when/why of the offending commit).
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Include a git-push example for creating a remote branch
From: Wincent Colaiuta @ 2007-09-06 10:50 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070906044408.GA588@spearce.org>
El 6/9/2007, a las 6:44, Shawn O. Pearce escribió:
> +git push origin master:refs/heads/experimental::
> + Create the branch `experimental` in the `origin` repository
> + by copying the current `master` branch. This form is usually
> + needed to create a new branch in the remote repository as
> + there is no `experimental` branch to match.
> +
A welcome addition; I was puzzled the first time I tried this myself
too.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Rework strbuf API and semantics.
From: Pierre Habouzit @ 2007-09-06 10:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtzq89kky.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]
On Thu, Sep 06, 2007 at 10:03:57AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > diff --git a/strbuf.c b/strbuf.c
> > ...
> > +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)
> > +{
> > + size_t len;
> > + va_list ap2;
> > +
> > + va_copy(ap2, ap);
> > +
> > + len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> > + if (len < 0) {
> > + len = 0;
> > + }
> > + if (len >= sb->alloc - sb->len) {
> > + strbuf_grow(sb, len);
> > + len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap2);
> > + if (len >= sb->alloc - sb->len) {
> > + len = sb->alloc - sb->len - 1;
> > + }
> > + }
> > + sb->len = sb->len + len;
> > + sb->buf[sb->len] = '\0';
> > }
>
> Hmmmmm... Didn't we recently had a patch that used va_copy()
> which was not available somewhere and was shot down?
>
> Instead of that nice inline strbuf_addf(), you may have to do
> something like:
>
> strbuf_addf(..., fmt, ...) {
> va_list ap;
>
> va_start(ap, fmt);
> len = vsnprintf(...);
> va_end(ap);
> if (len >= sb_avail(sb)) {
> strbuf_grow(sb, len);
> va_start(ap, fmt);
> len = vsnprintf(...);
> va_end(ap);
> if (len >= sb_avail(sb))
> gaah();
> }
> sb->len += len;
> sb->buf[sb->len] = '\0';
> }
I'll do that.
> > +ssize_t strbuf_read(struct strbuf *sb, int fd)
> > +{
> > + size_t oldlen = sb->len;
> > +
> > + for (;;) {
> > + ssize_t cnt;
> > +
> > + strbuf_grow(sb, 8192);
> > + cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
> > + if (cnt < 0) {
> > + sb->buf[sb->len = oldlen] = '\0';
>
> Assignment inside array subscript is very hard to read.
> Besides, what's the error semantics? On error, this behaves as
> if no bytes are read (i.e. partial read results in the initial
> round is lost forever)?
Yes that is the semantics, because it's how it was used everywhere in
git: either we were able to load all the data from the file descriptor,
or weren't able to and the next thing we do is to die().
Maybe I should call the function strbuf_xread() instead ?
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Andreas Ericsson @ 2007-09-06 10:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Dmitry Kakurin, Matthieu Moy, Git
In-Reply-To: <7vhcm8b0h8.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
>
>> Git is cheating a bit though. Its primary audience was (and is) the
>> various integrators working on the Linux kernel, all of whom are fairly
>> competent C programmers.
>
> Do we still have a huge overlap with the kernel people? I had
> an impression that patches from the kernel folks, with notable
> exception from a handful (you know who you are), have petered
> out rapidly after the first several weeks.
True, but the point I was trying to make is that because git is written
in C, for an audience who are extremely at home with that particular
language, it quickly attracted contributors.
git log --pretty=short | sed -n 's/^Author: \([^<]*\)<.*$/\1/p' | \
sort | uniq | wc -l
reports 355 unique lines, although some authors are mentioned twice
(Theodore Tso vs Theodore Ts'o). Cross-matching the kernel authors
with the git authors shows that git and linux have 111 developers
in common, again reporting some of them twice. A quick visual scan
shows the figure to be 106, assuming no two authors have the same
name (including email addresses produced more unique contributors as
people change email more often than they change name).
It's not unreasonable to say that git got at least 106 C-programmers
"for free" included in their userbase round about the same second
Linus went public with his intentions of managing the linux kernel
in git, all of which are obviously comfortable enough with C to
poke around in the kernel.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode
From: Dmitry V. Levin @ 2007-09-06 10:16 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20070906022539.GG18160@spearce.org>
[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]
On Wed, Sep 05, 2007 at 10:25:39PM -0400, Shawn O. Pearce wrote:
> "Dmitry V. Levin" <ldv@altlinux.org> wrote:
> > Do not commit an unchanged tree in non-merge mode.
>
> A laudable goal. git-gui also does this. Turns out the other
> checks within git-gui prevent the user from ever getting that far.
> I probably should remove the empty tree check as it costs CPU time
> to get the old tree. But I'd rather have the safety check.
>
> > The idea is that amend should not commit an unchanged tree,
> > one should just remove the top commit using git-reset instead.
>
> NO. `git commit --amend` is *often* used for fixing the commit
> message.
You see, my proposed change does not affect this usage case at all.
> Or adding additional detail.
If that "additional" detail just undoes the latest commit, why should
"git commit --amend" welcome such thing? I did not get your pint here.
> Forcing the user to do
> a `git reset --soft HEAD^ && git commit --amend` just because
> you don't want git-commit to make an "empty commit" (which it
> doesn't usually like to do now anyway!) is a major step back
> in functionality.
I suppose that helping users to avoid doing really stupid things
does not look as a major step back in functionality, just otherwise.
> > + current_tree="$(git cat-file commit "$current${amend:+^}" 2>/dev/null |
> > + sed -e '/^tree \+/!d' -e 's///' -e q)"
>
> The better way to get the old tree would be this:
>
> current_tree="$(git rev-parse "$current${amend:+^}^{tree}" 2>/dev/null
>
> as it avoids the tool from needing to know about the internal
> representation of a commit object. It also avoids an entire
> fork+exec of a sed process.
Agreed.
> > + if test "$tree" = "$current_tree"
> > + then
> > + echo >&2 "nothing to commit${amend:+ (use \"git reset HEAD^\" to remove the top commit)}"
>
> That message is a bad idea. Doing a mixed mode reset will also
> reset the index, causing the user to lose any changes that had
> already been staged. This may actually be difficult for him/her to
> recover from if they have used `git add -i` or git-gui to stage only
> certain hunks of files, or if their working tree has been further
> modified after the commit but they want to go back and amend the
> message only of the prior commit.
Would "git reset --soft HEAD^" advice be better than first one?
Could you suggest a better message, please?
--
ldv
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [announce] colgit: manage git repository collections
From: martin f krafft @ 2007-09-06 10:11 UTC (permalink / raw)
To: git discussion list; +Cc: home in vcs discussion list
[-- Attachment #1: Type: text/plain, Size: 3273 bytes --]
[Ob crosspost: please keep vcs-home on Cc all times. Please remove
git mailing list from Cc when the reply isn't enough about git]
Dear colleagues,
Last night, while Andy Roddick was giving his best against bored
world champion Roger Federer (and still lost after a game of many
crazy shots), I reached what I think is a milestone in my attempt to
let git manage my home directory across several machines. I called
it colgit, which attempts to hint at "collection of gits", and this
is the birth announcement of this shell script.
http://git.madduck.net/v/bin/colgit.git
The problem I am trying to solve is that I maintain different
projects in different repos, including scripts in ~/.bin and
configuration files, and that I use multiple computers on a daily
basis. Moreover, I might happen upon a new computer which I have to
use for a few days and would like to be able to set up my account
easily.
With SVN, I'd have a repository for each machine, which existed only
of svn:externals references pulling in other repos and thus
assembling my home directory. git-submodule isn't quite designed for
this sort of stuff, and thus I came up with colgit.
The idea is simple: ~ is a git repository with a .colgit/ directory,
which hosts a hierarchy of directories holding a selection of files
from .git directories. For instance, ~/.colgit/.etc/mutt/config is
the git-config file for the repository that I want to have in
~/.etc/mutt. Since all of ~/.colgit is checked in to the repo in ~,
I can easily clone that for another machine, or branch from it and
add yet other repositories, or remove some.
Each directory in ~/.colgit can also hold hooks, description, and
info/exclude, which are used to seed the repository in the plain
~ hierarchy. Right now, the design requires a central repository and
appropriate remote.origin.*/branch.master.merge entries in the
config file so that it can initialise repositories on new machines.
colgit currently is still very much a hack, but it already sports
the following commands:
update/init: for each leaf directory in ~/.colgit, create the
corresponding repository in ~, seed it, and run git-pull. If the
respository already exists, just pull.
register: given an existing repository somewhere under ~, obtain
the relevant config files from its $GIT_DIR and populate the
corresponding directory under ~/.colgit.
status: for each leaf directory in ~/.colgit, query the
corresponding repository in ~ and note to stdout if it has local
changes
I also envision the following commands:
add: given a repository URL and a local path, clone the repository
and run register
do: run the given git commands over each known repository
The process of initialising an account on a remote machine thus
becomes:
git clone -n ssh://.../machines/base temp && mv temp/.git ~
cd ~ && git checkout HEAD
...
colgit init
This is release early release often, so it's far from perfect and
and probably buggy. But suggestions and patches are welcome!
--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
# vim:tw=70
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Rework strbuf API and semantics.
From: Junio C Hamano @ 2007-09-06 10:03 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <11890199232110-git-send-email-madcoder@debian.org>
Pierre Habouzit <madcoder@debian.org> writes:
> diff --git a/strbuf.c b/strbuf.c
> ...
> +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)
> +{
> + size_t len;
> + va_list ap2;
> +
> + va_copy(ap2, ap);
> +
> + len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> + if (len < 0) {
> + len = 0;
> + }
> + if (len >= sb->alloc - sb->len) {
> + strbuf_grow(sb, len);
> + len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap2);
> + if (len >= sb->alloc - sb->len) {
> + len = sb->alloc - sb->len - 1;
> + }
> + }
> + sb->len = sb->len + len;
> + sb->buf[sb->len] = '\0';
> }
Hmmmmm... Didn't we recently had a patch that used va_copy()
which was not available somewhere and was shot down?
Instead of that nice inline strbuf_addf(), you may have to do
something like:
strbuf_addf(..., fmt, ...) {
va_list ap;
va_start(ap, fmt);
len = vsnprintf(...);
va_end(ap);
if (len >= sb_avail(sb)) {
strbuf_grow(sb, len);
va_start(ap, fmt);
len = vsnprintf(...);
va_end(ap);
if (len >= sb_avail(sb))
gaah();
}
sb->len += len;
sb->buf[sb->len] = '\0';
}
> +ssize_t strbuf_read(struct strbuf *sb, int fd)
> +{
> + size_t oldlen = sb->len;
> +
> + for (;;) {
> + ssize_t cnt;
> +
> + strbuf_grow(sb, 8192);
> + cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
> + if (cnt < 0) {
> + sb->buf[sb->len = oldlen] = '\0';
Assignment inside array subscript is very hard to read.
Besides, what's the error semantics? On error, this behaves as
if no bytes are read (i.e. partial read results in the initial
round is lost forever)?
^ permalink raw reply
* Re: [PATCH 1/3] git-gui/Makefile: Replace libdir with gitgui_libdir
From: Dmitry V. Levin @ 2007-09-06 10:00 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20070906023227.GH18160@spearce.org>
[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]
On Wed, Sep 05, 2007 at 10:32:27PM -0400, Shawn O. Pearce wrote:
> "Dmitry V. Levin" <ldv@altlinux.org> wrote:
> > On GNU/Linux, libdir is used to mean "/usr/lib or /usr/lib64"
> > depending on architecture. Different libdir meaning breaks
> > idiomatic expressions like rpm specfile "make libdir=%_libdir".
> >
> > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> > ---
> > git-gui/Makefile | 16 ++++++++--------
> > 1 files changed, 8 insertions(+), 8 deletions(-)
>
> Although I could apply this with `am -3` I'm NACK'ing this right
> now because...
[...]
> git-gui is its own project with its own Makefile. Junio includes
> it in git.git to help widen its audience, and because it is quite
> portable and easy for him to include. In the future git-gui will
> become a proper subproject of git.git.
The idea is that git-gui's libdir is not a traditional arch-dependent
libdir's subdirectory, but rather arch-independent datadir's subdirectory.
That is, I see no reason to call it libdir even in standalone project.
> If you want to define libdir in git's toplevel Makefile *and*
> that definition is being exported down into git-gui's Makefile
Yes, that was my first motivation, but the issue appears to be a bit more
complex.
--
ldv
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: David Kastrup @ 2007-09-06 9:52 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Shawn O. Pearce, Dmitry Kakurin, Matthieu Moy, Git
In-Reply-To: <46DFC490.3060200@op5.se>
Andreas Ericsson <ae@op5.se> writes:
> Shawn O. Pearce wrote:
>> Dmitry Kakurin <dmitry.kakurin@gmail.com> wrote:
>>> When I first looked at Git source code two things struck me as odd:
>>> 1. Pure C as opposed to C++. No idea why. Please don't talk about
>>> portability, it's BS.
>>
>> It is also a relatively simple language that
>> a large number of open source programmers know. This makes it easy
>> for them to get involved in the project.
>
>
> This is important. Git contains code from more than 300 people. I'm
> guessing you could cut that number by 2/3 if it had been written in
> C++.
C++ is a language without design discipline. Its set of features and
syntactic elements is incontingent (for example, its templates started
as a ripoff of Ada generics which would have been ok except for the
completely braindead idea of taking the Ada angle bracket restriction
syntax along with it), and it is the task of each programmer to choose
a sane and manageable subset and style, and implement using that. As
a consequence, every C++ programmer writes his own personal dialect of
C++, and we have about 20 different incompatible implementations of
multidimensional numeric arrays, making a complete mockery of the
"code reuse" mantra: C++ _projects_ can't actually usefully achieve
"multiple inheritance" on a design/meta level: once you start with one
non-trivial design, fitting other separately evolved components with a
different style causes retrofitting nightmares.
So going to C++ means cutting down the amount of people who find
themselves comfortable with the actual design and layout down to maybe
10% of those who would actually feel ok with the actual _algorithms_
employed.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH 3/9] Add strbuf_printf() to do formatted printing to a strbuf.
From: Pierre Habouzit @ 2007-09-06 9:50 UTC (permalink / raw)
To: Junio C Hamano, Kristian Høgsberg, git
In-Reply-To: <20070906094323.GB8451@artemis.corp>
[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]
On Thu, Sep 06, 2007 at 09:43:23AM +0000, Pierre Habouzit wrote:
> On Thu, Sep 06, 2007 at 08:55:26AM +0000, Junio C Hamano wrote:
> > As you noted in your follow-up message, this one has overlaps
> > with the other strbuf series. I could adjust them if I wanted
> > to, but I do not have time for it right now. I might try over
> > the weekend but no promises.
>
> This one is exactly the same as the one that follows, I corrected a
> typo in the comment of the commit, and I forgot to cleanse my tree
> before running format-patch again, so you have the "second" patch twice
> (name of the patch changed, hence the new one did not replaced the
> previous, but created a 0002-foo.patch instead of overwriting the
> 0002-bar.patch).
>
> You just must drop the one I followup-ed to.
err sorry, I mixed up with another thread. Did not had coffee yet :/
Just nvm
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Rework strbuf API and semantics.
From: Pierre Habouzit @ 2007-09-06 9:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkbkb0na.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 3634 bytes --]
On Thu, Sep 06, 2007 at 09:31:37AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > A strbuf can be used to store byte arrays, or as an extended string
> > library. The `buf' member can be passed to any C legacy string function,
> > because strbuf operations always ensure there is a terminating \0 at the end
> > of the buffer, not accounted in the `len' field of the structure.
> >
> > A strbuf can be used to generate a string/buffer whose final size is not
> > really known, and then "strbuf_detach" can be used to get the built buffer,
> > and keep the wrapping "strbuf" structure usable for further work again.
> >
> > Other interesting feature: strbuf_grow(sb, size) ensure that there is
> > enough allocated space in `sb' to put `size' new octets of data in the
> > buffer. It helps avoiding reallocating data for nothing when the problem the
> > strbuf helps to solve has a known typical size.
>
> "Rework API semantics" needs to be accompanied with an API
> description, perhaps at the beginning of each externally
> visible function.
>
> Also the commit log message needs to explain what the old
> semantics was and what the improved one is, to highlight the
> changes needed to the callers. Especially...
>
> > @@ -1657,11 +1656,11 @@ static void *cmd_data (size_t *size)
> > if (term_len == command_buf.len
> > && !strcmp(term, command_buf.buf))
> > break;
> > - ALLOC_GROW(buffer, length + command_buf.len, sz);
> > + ALLOC_GROW(buffer, length + command_buf.len + 1, sz);
> > memcpy(buffer + length,
> > command_buf.buf,
> > - command_buf.len - 1);
> > - length += command_buf.len - 1;
> > + command_buf.len);
> > + length += command_buf.len;
> > buffer[length++] = '\n';
> > }
> > free(term);
>
> .... it is not all obvious why these off-by-one changes are
> needed without such a description. The other hunks in this
> patch to this file are all such changes.
Yes, as I suppose you know, but I state it here again so that
everybody understands, before strbuf's were merely a byte array, not
necessarily NUL-terminated. Hence many parts of the code that wanted to
pass the buffer to str* functions had to manually insert a NUL, hence it
was accounted in the length of the buffer.
Now, we always have a NUL after the "official" end of the buffer, so
it's not needed anymore. The off-by-ones are just that. The hunk you
quote is one where git's code was messing with strbufs internals
directly, so the ALLOC_GROW has to take the 1 octed needed to maintain
the internal invariant. Though, the patch after this one rewrites the
hunk to use strbuf's API's.
>
> > -static void inline strbuf_add(struct strbuf *sb, int ch) {
>
> > +static inline void strbuf_addch(struct strbuf *sb, size_t c) {
> > + strbuf_grow(sb, 1);
> > + sb->buf[sb->len++] = c;
> > + sb->buf[sb->len] = '\0';
> > +}
>
> You certainly did not mean size_t wide characters.
Oh boy, now I've been red-handed of :%s/\<int\>/\<size_t\>/ :)
I'll repost a _clean_ patch series soon with those things fixed, and
the wrong overflow test (that should be a >= and not a >) as well, as we
discussed it on IRC before.
Oh and FWIW I believe the details of the NUL always after the buffer
array has to be in strbuf.h and not in the commit comment, maybe I'll
put it in both to make everybody happy.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 3/9] Add strbuf_printf() to do formatted printing to a strbuf.
From: Pierre Habouzit @ 2007-09-06 9:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristian Høgsberg, git
In-Reply-To: <7vwsv4b2bl.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 890 bytes --]
On Thu, Sep 06, 2007 at 08:55:26AM +0000, Junio C Hamano wrote:
> As you noted in your follow-up message, this one has overlaps
> with the other strbuf series. I could adjust them if I wanted
> to, but I do not have time for it right now. I might try over
> the weekend but no promises.
This one is exactly the same as the one that follows, I corrected a
typo in the comment of the commit, and I forgot to cleanse my tree
before running format-patch again, so you have the "second" patch twice
(name of the patch changed, hence the new one did not replaced the
previous, but created a 0002-foo.patch instead of overwriting the
0002-bar.patch).
You just must drop the one I followup-ed to.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Junio C Hamano @ 2007-09-06 9:35 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Shawn O. Pearce, Dmitry Kakurin, Matthieu Moy, Git
In-Reply-To: <46DFC490.3060200@op5.se>
Andreas Ericsson <ae@op5.se> writes:
> Git is cheating a bit though. Its primary audience was (and is) the
> various integrators working on the Linux kernel, all of whom are fairly
> competent C programmers.
Do we still have a huge overlap with the kernel people? I had
an impression that patches from the kernel folks, with notable
exception from a handful (you know who you are), have petered
out rapidly after the first several weeks.
^ permalink raw reply
* Re: [PATCH] Rework strbuf API and semantics.
From: Junio C Hamano @ 2007-09-06 9:31 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <11890199232110-git-send-email-madcoder@debian.org>
Pierre Habouzit <madcoder@debian.org> writes:
> A strbuf can be used to store byte arrays, or as an extended string
> library. The `buf' member can be passed to any C legacy string function,
> because strbuf operations always ensure there is a terminating \0 at the end
> of the buffer, not accounted in the `len' field of the structure.
>
> A strbuf can be used to generate a string/buffer whose final size is not
> really known, and then "strbuf_detach" can be used to get the built buffer,
> and keep the wrapping "strbuf" structure usable for further work again.
>
> Other interesting feature: strbuf_grow(sb, size) ensure that there is
> enough allocated space in `sb' to put `size' new octets of data in the
> buffer. It helps avoiding reallocating data for nothing when the problem the
> strbuf helps to solve has a known typical size.
"Rework API semantics" needs to be accompanied with an API
description, perhaps at the beginning of each externally
visible function.
Also the commit log message needs to explain what the old
semantics was and what the improved one is, to highlight the
changes needed to the callers. Especially...
> @@ -1657,11 +1656,11 @@ static void *cmd_data (size_t *size)
> if (term_len == command_buf.len
> && !strcmp(term, command_buf.buf))
> break;
> - ALLOC_GROW(buffer, length + command_buf.len, sz);
> + ALLOC_GROW(buffer, length + command_buf.len + 1, sz);
> memcpy(buffer + length,
> command_buf.buf,
> - command_buf.len - 1);
> - length += command_buf.len - 1;
> + command_buf.len);
> + length += command_buf.len;
> buffer[length++] = '\n';
> }
> free(term);
... it is not all obvious why these off-by-one changes are
needed without such a description. The other hunks in this
patch to this file are all such changes.
> -static void inline strbuf_add(struct strbuf *sb, int ch) {
> +static inline void strbuf_addch(struct strbuf *sb, size_t c) {
> + strbuf_grow(sb, 1);
> + sb->buf[sb->len++] = c;
> + sb->buf[sb->len] = '\0';
> +}
You certainly did not mean size_t wide characters.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox