promisor-remote: refactor should_accept_remote() control flow

A previous commit made sure we now reject empty URLs early at parse
time. This makes the existing warning() in case a remote URL is NULL
or empty very unlikely to be useful.

In future work, we also plan to add URL-based acceptance logic into
should_accept_remote().

To adapt to previous changes and prepare for upcoming changes, let's
restructure the control flow in should_accept_remote().

Concretely, let's:

 - Replace the warning() in case of an empty URL with a BUG(), as a
   previous commit made sure empty URLs are rejected early at parse
   time.

 - Move that modified empty-URL check to the very top of the function,
   so that every acceptance mode, instead of only ACCEPT_KNOWN_URL, is
   covered.

 - Invert the URL comparison: instead of returning on match and
   warning on mismatch, return early on mismatch and let the match
   case fall through. This opens a single exit path at the bottom of
   the function for future commits to extend.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/promisor-remote.c b/promisor-remote.c
index 8322349ae8ba87e9b1e0459fd1a4ffbbceb8c982..5860a3d3f36e09015fe74606dd366c8556ae34c5 100644 (file)
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -651,6 +651,11 @@ static int should_accept_remote(enum accept_promisor accept,
        const char *remote_name = advertised->name;
        const char *remote_url = advertised->url;
 
+       if (!remote_url || !*remote_url)
+               BUG("no or empty URL advertised for remote '%s'; "
+                   "this remote should have been rejected earlier",
+                   remote_name);
+
        if (accept == ACCEPT_ALL)
                return all_fields_match(advertised, config_info, NULL);
 
@@ -669,19 +674,14 @@ static int should_accept_remote(enum accept_promisor accept,
        if (accept != ACCEPT_KNOWN_URL)
                BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
 
-       if (!remote_url || !*remote_url) {
-               warning(_("no or empty URL advertised for remote '%s', "
-                         "ignoring this remote"), remote_name);
+       if (strcmp(p->url, remote_url)) {
+               warning(_("known remote named '%s' but with URL '%s' instead of '%s', "
+                         "ignoring this remote"),
+                       remote_name, p->url, remote_url);
                return 0;
        }
 
-       if (!strcmp(p->url, remote_url))
-               return all_fields_match(advertised, config_info, p);
-
-       warning(_("known remote named '%s' but with URL '%s' instead of '%s', "
-                 "ignoring this remote"), remote_name, p->url, remote_url);
-
-       return 0;
+       return all_fields_match(advertised, config_info, p);
 }
 
 static int skip_field_name_prefix(const char *elem, const char *field_name, const char **value)