Date: Sat, 6 Nov 2010 12:01:28 -0500 obj_pool is inherently global and does not use the standard growing factor alloc_nr, which makes it feel out of place in the git codebase. Plus it is overkill for this application: all that is needed is a buffer that can grow between requests to accomodate larger strings. Use a strbuf instead. As a side effect, this improves the error handling: allocation failures will result in a clean exit instead of segfaults. It would be nice to add a test case (using ulimit or failmalloc) but that can wait for another day. Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> --- Requires jn/thinner-wrapper (from master) if contrib/svn-fe/svn-fe is to build without linking to libz et al. The initial size of the per-line buffer shrinks from 4096 to 0 (well, maybe 16 or so). strbuf_fread is not inline. I haven't looked into the effect on performance from these changes. I find obj_pool tricky to use correctly (see 3c93983, vcs-svn: fix intermittent repo_tree corruption, 2010-12-05 for example) so I look forward to eliminating obj_pool from the vcs-svn/ dir altogether. Excitingly enough, David has already done that, it seems[1]. [1] git://github.com/barrbrain/git.git vcs-svn-incremental vcs-svn/line_buffer.c | 16 ++++++---------- 1 files changed, 6 insertions(+), 10 deletions(-) diff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c index f22c94f..6f32f28 100644 --- a/vcs-svn/line_buffer.c +++ b/vcs-svn/line_buffer.c @@ -5,15 +5,13 @@ #include "git-compat-util.h" #include "line_buffer.h" -#include "obj_pool.h" +#include "strbuf.h" #define LINE_BUFFER_LEN 10000 #define COPY_BUFFER_LEN 4096 -/* Create memory pool for char sequence of known length */ -obj_pool_gen(blob, char, 4096) - static char line_buffer[LINE_BUFFER_LEN]; +static struct strbuf blob_buffer = STRBUF_INIT; static FILE *infile; int buffer_init(const char *filename) @@ -58,11 +56,9 @@ char *buffer_read_line(void) char *buffer_read_string(uint32_t len) { - char *s; - blob_free(blob_pool.size); - s = blob_pointer(blob_alloc(len + 1)); - s[fread(s, 1, len, infile)] = '\0'; - return ferror(infile) ? NULL : s; + strbuf_reset(&blob_buffer); + strbuf_fread(&blob_buffer, len, infile); + return ferror(infile) ? NULL : blob_buffer.buf; } void buffer_copy_bytes(uint32_t len) @@ -94,5 +90,5 @@ void buffer_skip_bytes(uint32_t len) void buffer_reset(void) { - blob_reset(); + strbuf_release(&blob_buffer); } -- 1.7.2.3.554.gc9b5c.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html