diff options
author | Jeff King <peff@peff.net> | 2022-12-13 13:52:58 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2022-12-13 16:08:52 +0300 |
commit | a31cfe32834423c56911705f926077213c5f5f82 (patch) | |
tree | ee3d1a89afc346245cd868083b61cd1526303039 | |
parent | 8706a59933d09354c5e3eb09a543453655a97183 (diff) |
server_supports_v2(): use a separate function for die_on_error
The server_supports_v2() helper lets a caller find out if the server
supports a feature, and will optionally die if it's not supported. This
makes the return value confusing, as it's only meaningful when the
function is not asked to die.
Coverity flagged a new call like:
/* check that we support "foo" */
server_supports_v2("foo", 1);
complaining that we usually checked the return value, but this time we
didn't. But this call is correct, and other ones that did:
if (server_supports_v2("foo", 1))
do_something_with_foo();
are "wrong", in the sense that we know the conditional will always be
true (but there's no bug; the code is simply misleading).
Let's split the "die" behavior into its own function which returns void,
and modify each caller to use the correct one.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r-- | connect.c | 21 | ||||
-rw-r--r-- | connect.h | 3 | ||||
-rw-r--r-- | fetch-pack.c | 12 |
3 files changed, 20 insertions, 16 deletions
@@ -66,7 +66,7 @@ static NORETURN void die_initial_contact(int unexpected) } /* Checks if the server supports the capability 'c' */ -int server_supports_v2(const char *c, int die_on_error) +int server_supports_v2(const char *c) { int i; @@ -76,11 +76,13 @@ int server_supports_v2(const char *c, int die_on_error) (!*out || *out == '=')) return 1; } + return 0; +} - if (die_on_error) +void ensure_server_supports_v2(const char *c) +{ + if (!server_supports_v2(c)) die(_("server doesn't support '%s'"), c); - - return 0; } int server_feature_v2(const char *c, const char **v) @@ -477,7 +479,7 @@ static void send_capabilities(int fd_out, struct packet_reader *reader) { const char *hash_name; - if (server_supports_v2("agent", 0)) + if (server_supports_v2("agent")) packet_write_fmt(fd_out, "agent=%s", git_user_agent_sanitized()); if (server_feature_v2("object-format", &hash_name)) { @@ -504,17 +506,18 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, &transport_options->unborn_head_target : NULL; *list = NULL; - if (server_supports_v2("ls-refs", 1)) - packet_write_fmt(fd_out, "command=ls-refs\n"); + ensure_server_supports_v2("ls-refs"); + packet_write_fmt(fd_out, "command=ls-refs\n"); /* Send capabilities */ send_capabilities(fd_out, reader); - if (server_options && server_options->nr && - server_supports_v2("server-option", 1)) + if (server_options && server_options->nr) { + ensure_server_supports_v2("server-option"); for (i = 0; i < server_options->nr; i++) packet_write_fmt(fd_out, "server-option=%s", server_options->items[i].string); + } packet_delim(fd_out); /* When pushing we don't want to request the peeled tags */ @@ -20,7 +20,8 @@ enum protocol_version discover_version(struct packet_reader *reader); int server_supports_hash(const char *desired, int *feature_supported); const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset); -int server_supports_v2(const char *c, int die_on_error); +int server_supports_v2(const char *c); +void ensure_server_supports_v2(const char *c); int server_feature_v2(const char *c, const char **v); int server_supports_feature(const char *c, const char *feature, int die_on_error); diff --git a/fetch-pack.c b/fetch-pack.c index 998fc2fa1e..04016d1e32 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1317,15 +1317,15 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf, { const char *hash_name; - if (server_supports_v2("fetch", 1)) - packet_buf_write(req_buf, "command=fetch"); - if (server_supports_v2("agent", 0)) + ensure_server_supports_v2("fetch"); + packet_buf_write(req_buf, "command=fetch"); + if (server_supports_v2("agent")) packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized()); - if (advertise_sid && server_supports_v2("session-id", 0)) + if (advertise_sid && server_supports_v2("session-id")) packet_buf_write(req_buf, "session-id=%s", trace2_session_id()); - if (server_options && server_options->nr && - server_supports_v2("server-option", 1)) { + if (server_options && server_options->nr) { int i; + ensure_server_supports_v2("server-option"); for (i = 0; i < server_options->nr; i++) packet_buf_write(req_buf, "server-option=%s", server_options->items[i].string); |