git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/4] vcs-svn: replace buffer_read_string memory pool with a strbuf

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Dec 24, 2010, 08:17 UTC
Message-ID
<20101224081700.GC29681@burratino>
In-Reply-To
<20101224080505.GA29681@burratino>
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@gmail.com>
---
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
Previous: Jonathan NiederNext: Jonathan Nieder
Message 3 of 25 in “teach vcs-svn/line_buffer to handle multiple input files”
  1. 0/4 teach vcs-svn/line_buffer to handle multiple input filesJonathan Nieder, Dec 24, 2010
  2. 1/4 vcs-svn: eliminate global byte_bufferJonathan Nieder, Dec 24, 2010
  3. 2/4 vcs-svn: replace buffer_read_string memory pool with a strbufJonathan Nieder, Dec 24, 2010
  4. 3/4 vcs-svn: collect line_buffer data in a structJonathan Nieder, Dec 24, 2010
  5. 4/4 vcs-svn: teach line_buffer to handle multiple input filesJonathan Nieder, Dec 24, 2010
  6. Jonathan NiederJan 3, 2011
  7. 5/8 vcs-svn: make test-line-buffer input format more flexibleJonathan Nieder, Jan 3, 2011
  8. 6/8 tests: give vcs-svn/line_buffer its own test scriptJonathan Nieder, Jan 3, 2011
  9. 7/8 vcs-svn: tweak test-line-buffer to not assume line-oriented inputJonathan Nieder, Jan 3, 2011
  10. 8/8 t0081 (line-buffer): add buffering testsJonathan Nieder, Jan 3, 2011
  11. Jonathan NiederJan 3, 2011
  12. 12/12 line_buffer: more wrappers around stdio functionsJonathan Nieder, Jan 3, 2011
  13. 09/12 vcs-svn: add binary-safe read functionJonathan Nieder, Jan 3, 2011
  14. 10/12 vcs-svn: allow character-oriented inputJonathan Nieder, Jan 3, 2011
  15. 11/12 vcs-svn: allow input from file descriptorJonathan Nieder, Jan 3, 2011
  16. 12/12 vcs-svn: teach line_buffer about temporary filesJonathan Nieder, Jan 3, 2011
  17. vcs-svn: give control over temporary file namesJonathan Nieder, Jan 22, 2011
  18. [PULL svn-fe] fast-import 'ls', line-buffer changesJonathan Nieder, Feb 26, 2011
  19. David Michael BarrFeb 26, 2011
  20. Junio C HamanoFeb 28, 2011
  21. fast-import: make code "-Wpointer-arith" cleanJonathan Nieder, Feb 28, 2011
  22. Sverre RabbelierFeb 28, 2011
  23. Junio C HamanoFeb 28, 2011
  24. Jonathan NiederFeb 28, 2011
  25. Junio C HamanoMar 1, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.