Taylor Blau <me@xxxxxxxxxxxx> writes:
> static int graph_write(int argc, const char **argv)
> {
> - struct string_list *pack_indexes = NULL;
> + struct strbuf buf = STRBUF_INIT;
> + struct string_list pack_indexes;
I would strongly prefer this to be initialized here, not ...
> struct oidset commits = OIDSET_INIT;
> struct object_directory *odb = NULL;
> - struct string_list lines;
> int result = 0;
> enum commit_graph_write_flags flags = 0;
>
> @@ -209,44 +221,34 @@ static int graph_write(int argc, const char **argv)
> return 0;
> }
>
> - string_list_init(&lines, 0);
> - if (opts.stdin_packs || opts.stdin_commits) {
> - struct strbuf buf = STRBUF_INIT;
> + if (opts.stdin_packs) {
> + string_list_init(&pack_indexes, 0);
... down here. I know that the use of it is guarded with a ternary
(opts.stdin_packs ? &pack_indexes : NULL), but that is why you need
to UNLEAK() it, intead of unconditionally releasing the strbuf, at
the end of this function, no?
> while (strbuf_getline(&buf, stdin) != EOF)
> - string_list_append(&lines, strbuf_detach(&buf, NULL));
> + string_list_append(&pack_indexes,
> + strbuf_detach(&buf, NULL));
> + } else if (opts.stdin_commits) {
> + oidset_init(&commits, 0);
> + flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
>
> - if (opts.stdin_packs)
> - pack_indexes = &lines;
> - if (opts.stdin_commits) {
> - struct string_list_item *item;
> - oidset_init(&commits, lines.nr);
> - for_each_string_list_item(item, &lines) {
> - struct object_id oid;
> - const char *end;
> -
> - if (parse_oid_hex(item->string, &oid, &end)) {
> - error(_("unexpected non-hex object ID: "
> - "%s"), item->string);
> - return 1;
> - }
> -
> - oidset_insert(&commits, &oid);
> + while (strbuf_getline(&buf, stdin) != EOF) {
> + if (read_one_commit(&commits, buf.buf)) {
> + result = 1;
> + goto cleanup;
> }
> - flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
> }
> -
> - UNLEAK(buf);
> }
>
> if (write_commit_graph(odb,
> - pack_indexes,
> + opts.stdin_packs ? &pack_indexes : NULL,
> opts.stdin_commits ? &commits : NULL,
> flags,
> &split_opts))
> result = 1;
>
> - UNLEAK(lines);
> +cleanup:
> + UNLEAK(pack_indexes);
> + strbuf_release(&buf);
> return result;
> }
>
> --
> 2.26.0.113.ge9739cdccc
