Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> Change an UNLEAK() added in 0c4542738e6 (clone: free or UNLEAK further
> pointers when finished, 2021-03-14) to use a "to_free" pattern
> instead. In this case the "repo" can be either this absolute_pathdup()
> value, or in the "else if" branch seen in the context the the
> "argv[0]" argument to "main()".
>
> We can only free() the value in the former case, hence the "to_free"
> pattern.
Many other patches in the series, especially the ones that touch the
library-ish parts that can be called unbounded number of times, do
make sense, but this patch helps nobody. Not even the leak checker,
who should already be happy with the existing UNLEAK() marking. The
only thing it does is to free() a piece of memory obtained from a
one-time allocation that will be discarded upon program exit anyway.
If this were a freestanding patch done outside any series, I would
probably have not bothered to queue it. I'll be queuing as part of
the larger whole, but I am not all that impressed by this step and
the thinking that led to its inclusion in the series.
Thanks.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> builtin/clone.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index f518bb2dc1f..48156a4f2c2 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -892,6 +892,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> int is_bundle = 0, is_local;
> int reject_shallow = 0;
> const char *repo_name, *repo, *work_tree, *git_dir;
> + char *repo_to_free = NULL;
> char *path = NULL, *dir, *display_repo = NULL;
> int dest_exists, real_dest_exists = 0;
> const struct ref *refs, *remote_head;
> @@ -949,7 +950,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> path = get_repo_path(repo_name, &is_bundle);
> if (path) {
> FREE_AND_NULL(path);
> - repo = absolute_pathdup(repo_name);
> + repo = repo_to_free = absolute_pathdup(repo_name);
> } else if (strchr(repo_name, ':')) {
> repo = repo_name;
> display_repo = transport_anonymize_url(repo);
> @@ -1392,7 +1393,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> free(unborn_head);
> free(dir);
> free(path);
> - UNLEAK(repo);
> + free(repo_to_free);
> junk_mode = JUNK_LEAVE_ALL;
>
> transport_ls_refs_options_release(&transport_ls_refs_options);
