Re: [GSoC PATCH v5 2/5] repo: add the field references.format
From: Patrick Steinhardt <hidden>
Date: 2025-07-29 09:35:48
On Sun, Jul 27, 2025 at 02:51:07PM -0300, Lucas Seiki Oshiro wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin/repo.c b/builtin/repo.c index d4f01e35e2..02d5821c77 100644 --- a/builtin/repo.c +++ b/builtin/repo.c@@ -1,12 +1,90 @@ #include "builtin.h" #include "parse-options.h" +#include "refs.h" +#include "strbuf.h" -static int repo_info(int argc UNUSED, const char **argv UNUSED, - const char *prefix UNUSED, struct repository *repo UNUSED) +typedef int get_value_fn(struct repository *repo, struct strbuf *buf);
Nice, this now uses a strbuf as recommended.
quoted hunk ↗ jump to hunk
+struct field { + const char *key; + get_value_fn *get_value; +}; + +static int get_references_format(struct repository *repo, struct strbuf *buf) +{ + strbuf_addstr(buf, + ref_storage_format_to_name(repo->ref_storage_format)); + return 0; +}
And this prints into the buffer diretcly. Makes sense. [snip]
quoted hunk ↗ jump to hunk
+static int print_fields(int argc, const char **argv, struct repository *repo) +{ + const char *last = ""; + + QSORT(argv, argc, qsort_strcmp); + + for (int i = 0; i < argc; i++) { + get_value_fn *get_value; + const char *key = argv[i]; + struct strbuf value;
Let's declare the strbuf outside of the loop and `strbuf_reset()` it on every iteration.
quoted hunk ↗ jump to hunk
+ + if (!strcmp(key, last)) + continue; + + strbuf_init(&value, 64);
I don't think we should explicitly initialize it with a specific capacity. Let's just use `STRBUF_INIT`.
quoted hunk ↗ jump to hunk
+ get_value = get_value_fn_for_key(key); + + if (!get_value) { + strbuf_release(&value); + return error(_("key '%s' not found"), key); + } + + get_value(repo, &value); + printf("%s=%s\n", key, value.buf); + last = key; + strbuf_release(&value);
And the call to `strbuf_release()` should be moved to the end of this function so that we know to reuse the buffer. The above early return would then be converted into a `goto out` so that we have a common exit path where we know to clean up all resources. Patrick