Am 19.06.23 um 18:10 schrieb Taylor Blau:
> On Sat, Jun 17, 2023 at 10:41:44PM +0200, René Scharfe wrote:
>> diff --git a/strbuf.c b/strbuf.c
>> index 08eec8f1d8..a90b597da1 100644
>> --- a/strbuf.c
>> +++ b/strbuf.c
>> @@ -415,19 +415,24 @@ void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
>> strbuf_setlen(sb, sb->len + len);
>> }
>>
>> +int strbuf_expand_step(struct strbuf *sb, const char **formatp)
>> +{
>> + const char *format = *formatp;
>> + const char *percent = strchrnul(format, '%');
>
> Can format be NULL here? Obviously formatp is never NULL since it is
> `&s`, but we guarded the while loop in the pre-image with `while (*s)`
> and I am not sure that **formatp is always non-NULL here.
The "*s" in builtin/branch.c::quote_literal_for_format() that you quote
dereferences "s", so we'd get a segfault if it was NULL. A NULL (and
NUL) check would look like "while (s && *s)". The old code in strbuf.c
passes the format to strchrnul(), which can't handle NULL either. So
no, format must not be NULL here, and this is not a new requirement.
But now I noticed that builtin/branch.c::quote_literal_for_format()
only ever gets called with "" or "remotes/", none of which needs
quoting. We could drop this function entirely, and add it back when
needed, if ever. But that's out of the scope of this series.
René
