Re: [PATCH 07/17] commit: increase commit message buffer size
- From
Ben Peart <peartben@gmail.com>
- Date
- Jul 10, 2018, 18:08 UTC
- Message-ID
- <02c0682a-8cc9-be49-b646-00cbdb5783f3@gmail.com>
- In-Reply-To
- <20180709233952.GB535220@genre.crustytoothpaste.net>
On 7/9/2018 7:39 PM, brian m. carlson wrote:
Show 42 quoted lines
> On Mon, Jul 09, 2018 at 10:45:33AM -0700, Junio C Hamano wrote: >> Derrick Stolee <stolee@gmail.com> 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.