Re: [PATCH v3 2/2] builtin/pack-objects.c: introduce `pack.recentObjectsHook`
- From
Jeff King <peff@peff.net>
- Date
- May 12, 2023, 04:58 UTC
- Message-ID
- <20230512045853.GA2479240@coredump.intra.peff.net>
- In-Reply-To
- <2ce8a79fa4bf98947728da4e6e22304a2f203fac.1683847221.git.me@ttaylorr.com>
On Thu, May 11, 2023 at 07:20:37PM -0400, Taylor Blau wrote:
Show 32 quoted lines
> +static int run_one_pack_recent_objects_hook(struct oidset *set,
> + const char *args)
> +{
> + struct child_process cmd = CHILD_PROCESS_INIT;
> + struct strbuf buf = STRBUF_INIT;
> + FILE *out;
> + int ret = 0;
> +
> + cmd.use_shell = 1;
> + cmd.out = -1;
> +
> + strvec_push(&cmd.args, args);
> +
> + if (start_command(&cmd))
> + return -1;
> +
> + out = xfdopen(cmd.out, "r");
> + while (strbuf_getline(&buf, out) != EOF) {
> + struct object_id oid;
> + const char *rest;
> +
> + if (parse_oid_hex(buf.buf, &oid, &rest) || *rest) {
> + ret = error(_("invalid extra cruft tip: '%s'"), buf.buf);
> + goto done;
> + }
> +
> + oidset_insert(set, &oid);
> + }
> +
> + ret = finish_command(&cmd);
> +
> +done:I haven't looked closely at this whole patch yet (and I especially want to look at the new tests since this approach covers more cases), but I did notice that this version of the function still has the "we don't reap the child on parse failure" problem I described in:
https://lore.kernel.org/git/20230505221921.GE3321533@coredump.intra.peff.net/
I'll try to give the whole patch a more thorough review tomorrow.
-Peff