On Fri, Mar 31, 2023 at 03:59:04PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/bundle-uri.c b/bundle-uri.c
> index 177c1810402..56390651b92 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c
> @@ -792,6 +792,15 @@ int fetch_bundle_uri(struct repository *r, const char *uri,
>
> init_bundle_list(&list);
>
> + /*
> + * Do not fetch a NULL or empty bundle URI. An empty bundle URI
> + * could signal that a configured bundle URI has been disabled.
> + */
> + if (!uri || !*uri) {
> + result = 0;
> + goto cleanup;
> + }
Coverity flagged this "!uri"; it can never be true since we
unconditionally xstrdup(uri) at the top of the function, and we'd
segfault in that case.
I think the existing code that assumes a non-NULL uri is reasonable.
Even before the xstrdup() we'd have segfaulted when it was handed to
fetch_bundle_uri_internal().
It's not a huge deal, in the sense that you're just being overly
defensive, but the comment and conditional here are a little misleading.
-Peff
