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>

1 files changed, 2 insertions, 1 deletions

diff --git a/connect.h b/connect.h
index c53586e929..b26f7de784 100644
--- a/connect.h
+++ b/connect.h
@@ -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);