[RFC] strbuf's in builtin-apply
From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:34
I promised it, here it comes. It comes though with some possible
problems, so I'd like to see them discussed first.
builtin-apply.c was messing with its custom buffers a lot,
reallocating the buffers and so on, hence I had to create strbuf_embed
to have a strbuf-safe API doing exactly the same. Though, because of the
"I want an extra NUL in the end"-invariant, this can come with a quite
high price. in the second patch, there is a hunk:
- nsize = got;
- nbuf = convert_to_git(path, buf, &nsize);
- if (nbuf) {
- free(buf);
- *buf_p = nbuf;
- *alloc_p = nsize;
- *size_p = nsize;
- }
- return got != size;
+
+ nsize = buf->len;
+ nbuf = convert_to_git(path, buf->buf, &nsize);
+ if (nbuf)
+ strbuf_embed(buf, nbuf, nsize, nsize);
+ return 0;
Here, I've not been able to check if convert_to_git was in fact always
dealing with a NUL-terminated buffer (That would be in fact nsize+1) or
not, hence here this strbuf_embed will likely perform a realloc. I don't
know git enough to know if this can become an horrible burden though.
Another suspicious hunk is:
- data = (void*) fragment->patch;
[...]
case BINARY_LITERAL_DEFLATED:
- free(desc->buffer);
- desc->buffer = data;
- dst_size = fragment->size;
- break;
+ strbuf_embed(buf, fragment->patch, fragment->size, fragment->size);
+ return 0;
TTBOMK the ->patch pointer is a pointer inside a buffer, not a buffer
that has been malloc'ed (and there are code paths before my patch that
would still realloc the buffer so I don't think I introduce an issue).
It passes the test-suite without crashing, but well, maybe this should
be a copy instead.
The rest is pretty straightforward.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org Attachments
- (unnamed) [application/pgp-signature] 189 bytes