Christian Couder <christian.couder@xxxxxxxxx> writes:
> @@ -414,11 +439,15 @@ static int should_accept_remote(enum accept_promisor accept,
> return 0;
> }
>
> - if (!strcmp(urls->v[i], remote_url))
> + if (!p->url)
> + BUG("bad config_info (URL is NULL) for remote '%s'",
> + remote_name);
> +
> + if (!strcmp(p->url, remote_url))
> return 1;
The code seems to trust string_list enough that once an earlier call
to string_list_lookup() in this code path finds the entry for the
remote_name, item->util is assumed to be non-NULL and points at a
valid promisor_info instance. But it does not seem to be defensive
against p being NULL, but it does so for p->url being NULL, which
smells fishy. Once promisor_config_info_list() reads the data from
the configuration, do we ever update the contents and the risk of us
corrupting p->url while we are running is great enough to warrant
being defensive?
My inclination is to suggest removing the check and BUG() here, but
I may be missing something obvious (like "p->url gets NULL'ed out in
this function, but then such a remote is never checked with this
function because such and such logic in that function").
Thanks.
