promisor-remote: pass config entry to all_fields_match() directly
The `in_list == 0` path of all_fields_match() looks up the remote in `config_info` by `advertised->name` repeatedly, even though every caller in should_accept_remote() has already performed this lookup and holds the result in `p`. To avoid this useless work, let's replace the `int in_list` parameter with a `struct promisor_info *config_entry` pointer: - When NULL (ACCEPT_ALL mode): scan the whole `config_info` list, as the old `in_list == 1` path did. - When non-NULL: match against that single config entry directly, avoiding the redundant string_list_lookup() call. This removes the hidden dependency on `advertised->name` inside all_fields_match(), which would be wrong if in the future auto-configured remotes are implemented, as the local config name may differ from the server's advertised name. While at it, let's also add a comment before all_fields_match() and match_field_against_config() to help understand how things work and help avoid similar issues. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>maint
parent
8808e61fd3
commit
720b7c26c8
|
|
@ -575,6 +575,12 @@ enum accept_promisor {
|
|||
ACCEPT_ALL
|
||||
};
|
||||
|
||||
/*
|
||||
* Check if a specific field and its advertised value match the local
|
||||
* configuration of a given promisor remote.
|
||||
*
|
||||
* Returns 1 if they match, 0 otherwise.
|
||||
*/
|
||||
static int match_field_against_config(const char *field, const char *value,
|
||||
struct promisor_info *config_info)
|
||||
{
|
||||
|
|
@ -586,9 +592,18 @@ static int match_field_against_config(const char *field, const char *value,
|
|||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* Check that the advertised fields match the local configuration.
|
||||
*
|
||||
* When 'config_entry' is NULL (ACCEPT_ALL mode), every checked field
|
||||
* must match at least one remote in 'config_info'.
|
||||
*
|
||||
* When 'config_entry' points to a specific remote's config, the
|
||||
* checked fields are compared against that single remote only.
|
||||
*/
|
||||
static int all_fields_match(struct promisor_info *advertised,
|
||||
struct string_list *config_info,
|
||||
int in_list)
|
||||
struct promisor_info *config_entry)
|
||||
{
|
||||
struct string_list *fields = fields_checked();
|
||||
struct string_list_item *item_checked;
|
||||
|
|
@ -597,7 +612,6 @@ static int all_fields_match(struct promisor_info *advertised,
|
|||
int match = 0;
|
||||
const char *field = item_checked->string;
|
||||
const char *value = NULL;
|
||||
struct string_list_item *item;
|
||||
|
||||
if (!strcasecmp(field, promisor_field_filter))
|
||||
value = advertised->filter;
|
||||
|
|
@ -607,7 +621,11 @@ static int all_fields_match(struct promisor_info *advertised,
|
|||
if (!value)
|
||||
return 0;
|
||||
|
||||
if (in_list) {
|
||||
if (config_entry) {
|
||||
match = match_field_against_config(field, value,
|
||||
config_entry);
|
||||
} else {
|
||||
struct string_list_item *item;
|
||||
for_each_string_list_item(item, config_info) {
|
||||
struct promisor_info *p = item->util;
|
||||
if (match_field_against_config(field, value, p)) {
|
||||
|
|
@ -615,12 +633,6 @@ static int all_fields_match(struct promisor_info *advertised,
|
|||
break;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
item = string_list_lookup(config_info, advertised->name);
|
||||
if (item) {
|
||||
struct promisor_info *p = item->util;
|
||||
match = match_field_against_config(field, value, p);
|
||||
}
|
||||
}
|
||||
|
||||
if (!match)
|
||||
|
|
@ -640,7 +652,7 @@ static int should_accept_remote(enum accept_promisor accept,
|
|||
const char *remote_url = advertised->url;
|
||||
|
||||
if (accept == ACCEPT_ALL)
|
||||
return all_fields_match(advertised, config_info, 1);
|
||||
return all_fields_match(advertised, config_info, NULL);
|
||||
|
||||
/* Get config info for that promisor remote */
|
||||
item = string_list_lookup(config_info, remote_name);
|
||||
|
|
@ -652,7 +664,7 @@ static int should_accept_remote(enum accept_promisor accept,
|
|||
p = item->util;
|
||||
|
||||
if (accept == ACCEPT_KNOWN_NAME)
|
||||
return all_fields_match(advertised, config_info, 0);
|
||||
return all_fields_match(advertised, config_info, p);
|
||||
|
||||
if (accept != ACCEPT_KNOWN_URL)
|
||||
BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
|
||||
|
|
@ -663,7 +675,7 @@ static int should_accept_remote(enum accept_promisor accept,
|
|||
}
|
||||
|
||||
if (!strcmp(p->url, remote_url))
|
||||
return all_fields_match(advertised, config_info, 0);
|
||||
return all_fields_match(advertised, config_info, p);
|
||||
|
||||
warning(_("known remote named '%s' but with URL '%s' instead of '%s'"),
|
||||
remote_name, p->url, remote_url);
|
||||
|
|
|
|||
Loading…
Reference in New Issue