Re: [PATCH v2 00/21] object_id part 7
- From
Jeff King <peff@peff.net>
- Date
- Mar 28, 2017, 17:35 UTC
- Message-ID
- <20170328173536.ylwesrj7jbreztcy@sigill.intra.peff.net>
- In-Reply-To
- <20170328111315.i6rhhy4pj6hhf4bs@genre.crustytoothpaste.net>
On Tue, Mar 28, 2017 at 11:13:15AM +0000, brian m. carlson wrote:
Show 12 quoted lines
> > I suggested an additional cleanup around "linelen" in one patch. In the > > name of keeping the number of re-rolls sane, I'm OK if we skip that for > > now (the only reason I mentioned it at all is that you have to justify > > the caveat in the commit message; with the fix, that justification can > > go away). > > Let's leave it as it is, assuming Junio's okay with it. I can send in a > few more patches to clean that up and use skip_prefix that we can drop > on top and graduate separately. > > I think the justification is useful as it is, since it explains why we > no longer want to check that particular value for historical reasons.
I thought I'd knock this out quickly before I forgot about it. But it actually isn't so simple.
The main caller in read_head_info() does indeed just pass strlen(line) as the length in each case. But the cert parser really does need us to respect the line length. So we either have to pass it in, or tie off the string.
The latter looks something like the patch below (on top of a minor tweak around "eol" handling). It's sufficiently ugly that it may not count as an actual cleanup, though. I'm OK if we just drop the idea.
---
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 58de2a1a9..561a982e7 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1483,13 +1483,10 @@ static void execute_commands(struct command *commands, } static struct command **queue_command(struct command **tail, - const char *line, - int linelen) + const char *line) { struct object_id old_oid, new_oid; struct command *cmd; - const char *refname; - int reflen; const char *p; if (parse_oid_hex(line, &old_oid, &p) || @@ -1498,9 +1495,7 @@ static struct command **queue_command(struct command **tail, *p++ != ' ') die("protocol error: expected old/new/ref, got '%s'", line); - refname = p; - reflen = linelen - (p - line); - FLEX_ALLOC_MEM(cmd, ref_name, refname, reflen); + FLEX_ALLOC_STR(cmd, ref_name, p); oidcpy(&cmd->old_oid, &old_oid); oidcpy(&cmd->new_oid, &new_oid); *tail = cmd; @@ -1510,7 +1505,7 @@ static struct command **queue_command(struct command **tail, static void queue_commands_from_cert(struct command **tail, struct strbuf *push_cert) { - const char *boc, *eoc; + char *boc, *eoc; if (*tail) die("protocol error: got both push certificate and unsigned commands"); @@ -1523,10 +1518,17 @@ static void queue_commands_from_cert(struct command **tail, eoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len); while (boc < eoc) { - const char *eol = memchr(boc, '\n', eoc - boc); + char *eol = memchr(boc, '\n', eoc - boc); + char tmp; + if (!eol) eol = eoc; - tail = queue_command(tail, boc, eol - boc); + + tmp = *eol; + *eol = '\0'; + tail = queue_command(tail, boc); + *eol = tmp; + boc = eol + 1; } } @@ -1590,7 +1592,7 @@ static struct command *read_head_info(struct oid_array *shallow) continue; } - p = queue_command(p, line, linelen); + p = queue_command(p, line); } if (push_cert.len)