On Thu, May 11, 2023 at 07:20:37PM -0400, Taylor Blau wrote:
> +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@xxxxxxxxxxxxxxxxxxxxxxx/
I'll try to give the whole patch a more thorough review tomorrow.
-Peff
