On Thu, Mar 10 2022, brian m. carlson wrote:
> + size_t author_len, committer_len;
> + struct commit *this = NULL;
> + const char *orig_author = NULL, *orig_committer = NULL;
> + char *author = NULL, *committer = NULL;
> + const char *buffer = NULL;
> + unsigned long bufsize;
> + const char *p;
> + char *msg = NULL;
These shouldn't be initialized unless they really need to..
> + this = lookup_commit_reference(the_repository, &info->w_commit);
..and some are clobbered right away here, so all of these should not be initializzed.
> + buffer = get_commit_buffer(this, &bufsize);
> + orig_author = find_commit_header(buffer, "author", &author_len);
> + orig_committer = find_commit_header(buffer, "committer", &committer_len);
> + p = memmem(buffer, bufsize, "\n\n", 2);
...since by doing so we hide genuine "uninitialized"
warnings. E.g. "author_len" here isn't initialized, but is set by
find_commit_header(), but if that line was removed we'd warn below, but
not if it's initialized when the variables are declared..
> + for (size_t i = 0;; i++, nitems++) {
> + char buf[32];
> + int ret;
> +
> + if (nalloc <= i) {
> + size_t new = nalloc * 3 / 2 + 5;
> + items = xrealloc(items, new * sizeof(*items));
> + nalloc = new;
Can't we just use the usual ALLOC_GROW() pattern here?
> + }
> + snprintf(buf, sizeof(buf), "%zu", i);
Aren't the %z formats unportable (even with our newly found reliance on
more C99)? I vaguely recall trying them recently and the windows CI jobs
erroring...
> + for (ssize_t i = nitems - 1; i >= 0; i--) {
The ssize_t type can be really small (it's not a signed size_t), so this
is unportable, but in practice maybe it's OK...
In this case if you're just wanting to count down in a list maybe you
can use the pattern I used in 99d60545f87 (string-list API: change "nr"
and "alloc" to "size_t", 2022-03-07)?
> + if (ref) {
> + update_ref(NULL, ref, &prev->object.oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
> + } else {
> + puts(oid_to_hex(&prev->object.oid));
> + }
Nit: braces can be gone.
> +out:
> + for (size_t i = 0; i < nitems; i++) {
> + free_stash_info(&items[i]);
> + }
Ditto.
> +static int export_stash(int argc, const char **argv, const char *prefix)
> +{
> + int ret = 0;
> + int print = 0;
> + const char *ref = NULL;
> + struct option options[] = {
> + OPT_BOOL(0, "print", &print,
> + N_("print the object ID instead of writing it to a ref")),
> + OPT_STRING(0, "to-ref", &ref, "refname",
Needs _("refname")
> + N_("save the data to the given ref")),
> + OPT_END()
> + };
> +
> + argc = parse_options(argc, argv, prefix, options,
> + git_stash_export_usage,
> + PARSE_OPT_KEEP_DASHDASH);
> +
> + if (!(!!ref ^ print))
> + return error("exactly one of --print or --to-ref is required");
Cute, but maybe we can just use OPT_CMDMODE(), and if it's "to-ref"
shift one off argv afterwards (which'll be your &ref).
I.e. it'll give you the option incompatibility check, and without a new
translaed string.
Which, if we're keeping this version should use _().
