git/builtin/fetch-pack.c, branch jch

builtin/fetch-pack: cleanup before return error

2025-06-04T15:52:25Z

In builtin/fetch-pack.c:cmd_fetch_pack(), if finish_connect() failed, it returns error code without cleanup which cause memory leak. Add cleanup label before frees in the end of cmd_fetch_pack(), and add `goto cleanup` if finish_connect() failed. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Acked-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

oddballs: send usage() help text to standard output

2025-01-17T21:30:03Z

Using the show_usage_if_asked() helper we introduced earlier, fix callers of usage() that want to show the help text when explicitly asked by the end-user. The help text now goes to the standard output stream for them. The callers in this step are oddballs in that their invocations of usage() are *not* guarded by if (argc == 2 && !strcmp(argv[1], "-h") usage(...); There are (unnecessarily) being clever ones that do things like if (argc != 2 || !strcmp(argv[1], "-h") usage(...); to say "I know I take only one argument, so argc != 2 is always an error regardless of what is in argv[]. Ah, by the way, even if argc is 2, "-h" is a request for usage text, so we do the same". Some like "git var -h" just do not treat "-h" any specially, and let it take the same error code paths as a parameter error. Now we cannot do the same, so these callers are rewrittin to do the show_usage_and_exit_if_asked() first and then handle the usage error the way they used to. Acked-by: Jeff King Signed-off-by: Junio C Hamano

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

fetch-pack: clear pack lockfiles list

2024-09-25T17:24:54Z

If the --lock-pack option is passed (which it typically is when fetch-pack is used under the hood by smart-http), then we may end up with entries in our pack_lockfiles string_list. We need to clear them before returning to avoid a leak. In git-fetch this isn't a problem, since the same cleanup happens via transport_unlock_pack(). But the leak is detectable in t5551, which does http fetches. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

fetch-pack, send-pack: clean up shallow oid array

2024-09-25T17:24:53Z

When we call get_remote_heads() for protocol v0, that may populate the "shallow" oid_array, which must be cleaned up to avoid a leak at the program exit. The same problem exists for both fetch-pack and send-pack, but not for the usual transport.c code paths, since we already do this cleanup in disconnect_git(). Fixing this lets us mark t5542 as leak-free for the send-pack side, but fetch-pack will need some more fixes before we can do the same for t5539. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

fetch-pack: free object filter before exiting

2024-09-25T17:24:53Z

Our fetch_pack_args holds a filter_options struct that may be populated with allocated strings by the by the "--filter" command-line option. We must free it before exiting to avoid a leak when the program exits. The usual fetch code paths that use transport.c don't have the same leak, because we do the cleanup in disconnect_git(). Fixing this leak lets us mark t5500 as leak-free. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

fetch-pack: fix leaking sought refs

2024-09-25T17:24:52Z

When calling `fetch_pack()` the caller is expected to pass in a set of sought-after refs that they want to fetch. This array gets massaged to not contain duplicate entries, which is done by replacing duplicate refs with `NULL` pointers. This modifies the caller-provided array, and in case we do unset any pointers the caller now loses track of that ref and cannot free it anymore. Now the obvious fix would be to not only unset these pointers, but to also free their contents. But this doesn't work because callers continue to use those refs. Another potential solution would be to copy the array in `fetch_pack()` so that we dont modify the caller-provided one. But that doesn't work either because the NULL-ness of those entries is used by callers to skip over ref entries that we didn't even try to fetch in `report_unmatched_refs()`. Instead, we make it the responsibility of our callers to duplicate these arrays as needed. It ain't pretty, but it works to plug the memory leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'jc/pass-repo-to-builtins'

2024-09-23T17:35:09Z

The convention to calling into built-in command implementation has been updated to pass the repository, if known, together with the prefix value. * jc/pass-repo-to-builtins: add: pass in repo variable instead of global the_repository builtin: remove USE_THE_REPOSITORY for those without the_repository builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h builtin: add a repository parameter for builtin functions

builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h

2024-09-13T21:32:24Z

Instead of including USE_THE_REPOSITORY_VARIABLE by default on every builtin, remove it from builtin.h and add it to all the builtins that include builtin.h (by definition, that means all builtins/*.c). Also, remove the include statement for repository.h since it gets brought in through builtin.h. The next step will be to migrate each builtin from having to use the_repository. Signed-off-by: John Cai Signed-off-by: Junio C Hamano

builtin: add a repository parameter for builtin functions

2024-09-13T21:27:08Z

In order to reduce the usage of the global the_repository, add a parameter to builtin functions that will get passed a repository variable. This commit uses UNUSED on most of the builtin functions, as subsequent commits will modify the actual builtins to pass the repository parameter down. Signed-off-by: John Cai Signed-off-by: Junio C Hamano