diff mbox series

[4/6] fast-import: avoid awkward use of `strbuf_attach()`

Message ID 5db92b51c0363694c72b2de0c841449fa4e03f28.1587240635.git.martin.agren@gmail.com (mailing list archive)
State New, archived
Headers show
Series strbuf: simplify `strbuf_attach()` usage | expand

Commit Message

Martin Ågren April 18, 2020, 8:18 p.m. UTC
As explained in an earlier commit, per the documentation of
`strbuf_attach()`, it is incorrect to pass in the same value for `alloc`
as we do for `len`. But, and this was also explained earlier, doing so
is still ok-ish because we'll end up allocating a large enough buffer
under the hood.

But then one really has to wonder whether

  strbuf_attach(&sb, buf, size, size);

is any better than

  strbuf_reset(&sb);
  strbuf_add(&sb, buf, size);
  free(buf);

The latter is certainly a lot less subtle about what is going on, and if
we're lucky, the strbuf's allocated buffer is large enough that we won't
even need to allocate. So let's change to this more explicit form.

In short, this commit should not make things any worse.

Nearby commits are changing other callsites to pass in a larger 'alloc`
parameter. Maybe that's safe here, too -- I admit I don't quite follow
where this memory comes from. In the future, we could possibly switch
back to `strbuf_attach()` here after looking into the allocations in
more detail. The immediate reason for this commit is that we want to
simplify the usage of `strbuf_attach()`, and we won't be able to pass in
"size, size" any more.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 fast-import.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/fast-import.c b/fast-import.c
index 202dda11a6..7fd501c5cf 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2946,10 +2946,11 @@  static void cat_blob(struct object_entry *oe, struct object_id *oid)
 	cat_blob_write("\n", 1);
 	if (oe && oe->pack_id == pack_id) {
 		last_blob.offset = oe->idx.offset;
-		strbuf_attach(&last_blob.data, buf, size, size);
+		strbuf_reset(&last_blob.data);
+		strbuf_add(&last_blob.data, buf, size);
 		last_blob.depth = oe->depth;
-	} else
-		free(buf);
+	}
+	free(buf);
 }
 
 static void parse_get_mark(const char *p)