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 /connect.c | |
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>
Diffstat (limited to 'connect.c')
-rw-r--r-- | connect.c | 21 |
1 files changed, 12 insertions, 9 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 */ |