Re: [PATCH GSoC v15 05/13] fetch-pack: drop static `advertise_sid` variable
From: Junio C Hamano <hidden>
Date: 2026-07-01 18:19:54
Pablo Sabater [off-list ref] writes:
`write_fetch_command_and_capabilities()` is moved to `connect.c` in a
subsequent commit. To prepare for that, drop the static variable usage
of `advertise_sid`. Currently `advertise_sid` is used in two places:
1. In function `do_fetch_pack()`:
if (!server_supports("session_id"))
advertise_sid = 0;
2. In function `fetch_pack_config()`:
repo_config_get_bool("transfer.advertisesid", &advertise_sid);
Since `do_fetch_pack()` is only relevant for protocol v1, it can be
ignored because `write_fetch_command_and_capabilities()` is only used in
protocol v2.
About 2, call `repo_config_get_bool()` directly inside of the function.Puzzled. The patch does introduce a local on-stack variable with the same name, but does not remove the file-scope global one.
quoted hunk ↗ jump to hunk
diff --git a/fetch-pack.c b/fetch-pack.c index f13951d154..ad07603755 100644 --- a/fetch-pack.c +++ b/fetch-pack.c@@ -1380,6 +1380,9 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf, const struct string_list *server_options) { const char *hash_name; + int advertise_sid; + + repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
If there is no such configuration variable defined anywhere in the system, advertise_sid will be left uninitialied. Wouldn't it cause problems later. Initialize it to 0 (if the default is not to advertise, which I think is the case but please double check), and you would be OK, probably?
quoted hunk ↗ jump to hunk
ensure_server_supports_v2("fetch"); packet_buf_write(req_buf, "command=fetch");@@ -1395,7 +1398,7 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf, } if (server_feature_v2("object-format", &hash_name)) { - int hash_algo = hash_algo_by_name(hash_name); + const unsigned int hash_algo = hash_algo_by_name(hash_name); if (hash_algo_by_ptr(the_hash_algo) != hash_algo) die(_("mismatched algorithms: client %s; server %s"), the_hash_algo->name, hash_name);
While at it, change `hash_algo`'s type to match `hash_algo_by_name()`'s actual return type (`unsigned int`) and make it `const`.
This one makes sense, but is better left out to a separate step. Especially when the primary focus of the change is iffy (see above).