Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
> @@ -1781,26 +1775,41 @@ static void repack_local_links(void)
> struct object_id *oid;
> char *base_name;
We may want to give a meaningless NULL initialization to this
variable, due to false positive from a compliler.
> - if (!oidset_size(&local_links))
> + if (!oidset_size(&outgoing_links))
> return;
>
> - base_name = mkpathdup("%s/pack/pack", repo_get_object_directory(the_repository));
It used to be that it was really obvious that base_name is always
initialized. But now due to micro-optimization ...
> + oidset_iter_init(&outgoing_links, &iter);
> + while ((oid = oidset_iter_next(&iter))) {
> + struct object_info info = OBJECT_INFO_INIT;
> + if (oid_object_info_extended(the_repository, oid, &info, 0))
> + /* Missing; assume it is a promisor object */
> + continue;
> + if (info.whence == OI_PACKED && info.u.packed.pack->pack_promisor)
> + continue;
> ...
> + if (!cmd.args.nr) {
> + base_name = mkpathdup(
> + "%s/pack/pack",
> + repo_get_object_directory(the_repository));
... we lazily allocate only after we know we will run a command.
> + strvec_push(&cmd.args, "pack-objects");
> + strvec_push(&cmd.args,
> + "--exclude-promisor-objects-best-effort");
> + strvec_push(&cmd.args, base_name);
> + cmd.git_cmd = 1;
> + cmd.in = -1;
> + cmd.out = -1;
> + if (start_command(&cmd))
> + die(_("could not start pack-objects to repack local links"));
> + }
We know outgoing_links is not empty, so we know we will enter the
while() loop at least once, but it may be possible that all the
objects in the outgoing_links oidset end up to be missing or packed
in a promisor pack, hitting continue and never running the command.
> - oidset_iter_init(&local_links, &iter);
> - while ((oid = oidset_iter_next(&iter))) {
> if (write_in_full(cmd.in, oid_to_hex(oid), the_hash_algo->hexsz) < 0 ||
> write_in_full(cmd.in, "\n", 1) < 0)
> die(_("failed to feed local object to pack-objects"));
> }
> +
> + if (!cmd.args.nr)
> + return;
But then we have this early return, so from human-reader's point of
view, we will never hit free(base_name) at the end of this function.
But GCC used in the macOS build does not seem to realize it.
https://github.com/git/git/actions/runs/12152173257/job/33888089229#step:4:380
It may be safer to give a meaningless NULL as the initial value of
the variable.
Thanks.
