Web lists-archives.com

Re: [PATCH 07/17] commit: increase commit message buffer size






On 7/9/2018 7:39 PM, brian m. carlson wrote:
On Mon, Jul 09, 2018 at 10:45:33AM -0700, Junio C Hamano wrote:
Derrick Stolee <stolee@xxxxxxxxx> writes:

On 7/8/2018 7:36 PM, brian m. carlson wrote:
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a9a066dcfb..252f835bae 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1587,7 +1587,7 @@ static int log_ref_write_fd(int fd, const struct object_id *old_oid,
   	char *logrec;
     	msglen = msg ? strlen(msg) : 0;
-	maxlen = strlen(committer) + msglen + 100;
+	maxlen = strlen(committer) + msglen + 200;
   	logrec = xmalloc(maxlen);
   	len = xsnprintf(logrec, maxlen, "%s %s %s\n",
   			oid_to_hex(old_oid),

nit: 100 is not enough anymore, but wasn't a very descriptive
value. 200 may be enough now, but I'm not sure why.

200 is definitely enough.  Suppose we had a message consisting entirely
of SHA-1 hashes (5, at 20 bytes a piece).  If our new hash is 32 bytes
long, then it would require at most 160 bytes.

I only noticed this because the old code segfaulted.  My approach to
using a 32-byte hash was to set it up, do some basic tests, find out
what crashed, and fix it.  Most of this series is the basics necessary
to get the most rudimentary functionality out of a 32-byte Git,
excluding the index pieces, which are necessarily inelegant.

I didn't include them because there are other ways to implement the
changes which are more elegant in some ways and less elegant in other
ways, and I want to think more about it before I send them in.

As Brandon alludes to downthread, we probably should use strbuf for
things like this these days, so a preliminary clean-up to do so is
probably a welcome change to sneak in and rebase this series on top
of.

Sure, I agree that would be a better change, and I'm happy to reroll
with that.


I've put together a patch to update log_ref_write_fd() to use strbuf and will submit it shortly.