Re: [PATCH v2 14/15] bundle-uri: handle NULL values in bundle lists

[Derrick Stolee <stolee@gmail.com>]

On 11/10/2026 6:11 AM, Phillip Wood wrote:
> I'll defer to Stolee on what's the best behavior here, but it seems that in a couple of places we're erroring out on a NULL value when we would ignore an invalid value.
> 
> On 10/11/2026 07:36, Thomas Bachem via GitGitGadget wrote:
>> From: Thomas Bachem <mail@thomasbachem.com>

>> +            if (!value)
>> +                return config_error_nonbool(key);
> 
> Adding a bit more context below shows we ignore unknown heuristics, so do we really want to return an error here?

>>         if (!strcmp(subkey, "creationtoken")) {
>> +        if (!value)
>> +            return config_error_nonbool(key);
>>           if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
>>               warning(_("could not parse bundle list key %s with value '%s'"),
>>                   "creationToken", value);
>         return 0;
>     }
> 
> Here we ignore parse errors, so should we be returning an error on a NULL value?

I think that the bundle URI feature could be hardened to halt (safely,
with good error messages) instead of continuing in a best-effort way.
The cost to being so flexible is that a user can download partial
bundles that don't even fully install and then fetches too much data
from the Git remote.

But I also think that this series isn't the right place to do that.

You're right that this method becomes inconsistent with how it handles
certain parsing errors versus others, but we are upgrading from SEGFAULT
to error.

Thanks,
-Stolee