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>
Jeff King committed
Dec 13, 2022 at 05:52 UTC
a31cfe32834423c56911705f926077213c5f5f82
3 files changed
+20
-16
connect.c
+12
-9
@@ -66,7 +66,7 @@ static NORETURN void die_initial_contact(int unexpected)
66
}
67
68
/* Checks if the server supports the capability 'c' */
69
-int server_supports_v2(const char *c, int die_on_error)
69
+int server_supports_v2(const char *c)
70
{
71
int i;
72
@@ -76,11 +76,13 @@ int server_supports_v2(const char *c, int die_on_error)
76
(!*out || *out == '='))
77
return 1;
78
}
79
+ return 0;
80
+}
81
80
- if (die_on_error)
82
+void ensure_server_supports_v2(const char *c)
83
+{
84
+ if (!server_supports_v2(c))
85
die(_("server doesn't support '%s'"), c);
82
-
83
- return 0;
86
}
87
88
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)
479
{
480
const char *hash_name;
481
480
- if (server_supports_v2("agent", 0))
482
+ if (server_supports_v2("agent"))
483
packet_write_fmt(fd_out, "agent=%s", git_user_agent_sanitized());
484
485
if (server_feature_v2("object-format", &hash_name)) {
@@ -504,17 +506,18 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
506
&transport_options->unborn_head_target : NULL;
507
*list = NULL;
508
507
- if (server_supports_v2("ls-refs", 1))
508
- packet_write_fmt(fd_out, "command=ls-refs\n");
509
+ ensure_server_supports_v2("ls-refs");
510
+ packet_write_fmt(fd_out, "command=ls-refs\n");
511
512
/* Send capabilities */
513
send_capabilities(fd_out, reader);
514
513
- if (server_options && server_options->nr &&
514
- server_supports_v2("server-option", 1))
515
+ if (server_options && server_options->nr) {
516
+ ensure_server_supports_v2("server-option");
517
for (i = 0; i < server_options->nr; i++)
518
packet_write_fmt(fd_out, "server-option=%s",
519
server_options->items[i].string);
520
+ }
521
522
packet_delim(fd_out);
523
/* When pushing we don't want to request the peeled tags */
connect.h
+2
-1
@@ -20,7 +20,8 @@ enum protocol_version discover_version(struct packet_reader *reader);
20
21
int server_supports_hash(const char *desired, int *feature_supported);
22
const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset);
23
-int server_supports_v2(const char *c, int die_on_error);
23
+int server_supports_v2(const char *c);
24
+void ensure_server_supports_v2(const char *c);
25
int server_feature_v2(const char *c, const char **v);
26
int server_supports_feature(const char *c, const char *feature,
27
int die_on_error);
fetch-pack.c
+6
-6
@@ -1317,15 +1317,15 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
1317
{
1318
const char *hash_name;
1319
1320
- if (server_supports_v2("fetch", 1))
1321
- packet_buf_write(req_buf, "command=fetch");
1322
- if (server_supports_v2("agent", 0))
1320
+ ensure_server_supports_v2("fetch");
1321
+ packet_buf_write(req_buf, "command=fetch");
1322
+ if (server_supports_v2("agent"))
1323
packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
1324
- if (advertise_sid && server_supports_v2("session-id", 0))
1324
+ if (advertise_sid && server_supports_v2("session-id"))
1325
packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
1326
- if (server_options && server_options->nr &&
1327
- server_supports_v2("server-option", 1)) {
1326
+ if (server_options && server_options->nr) {
1327
int i;
1328
+ ensure_server_supports_v2("server-option");
1329
for (i = 0; i < server_options->nr; i++)
1330
packet_buf_write(req_buf, "server-option=%s",
1331
server_options->items[i].string);