Subject: Re: [PATCH 12/20] http-backend.c: fix cmd_main() memory leak, refactor reg{exec,free}()

From: Junio C Hamano <[email-masked]>
Date: Thu, 29 Dec 2022 16:32:59 +0900

René Scharfe <[email-masked]> writes:

>> diff --git a/http-backend.c b/http-backend.c
>> index 6eb3b2fe51c..9bb63c458b1 100644
>> --- a/http-backend.c
>> +++ b/http-backend.c
>> @@ -759,10 +759,14 @@ int cmd_main(int argc, const char **argv)
>>  		struct service_cmd *c = &services[i];
>>  		regex_t re;
>>  		regmatch_t out[1];
>> +		int ret;
>>
>>  		if (regcomp(&re, c->pattern, REG_EXTENDED))
>>  			die("Bogus regex in service table: %s", c->pattern);
>> -		if (!regexec(&re, dir, 1, out, 0)) {
>> +		ret = regexec(&re, dir, 1, out, 0);
>> +		regfree(&re);
>> +
>> +		if (!ret) {
>>  			size_t n;
>>
>
> ... i.e. right here.  But only after copying the offsets out of "out"!

"only after..."?  Do out[i].rm_eo and out[i].rm_so become invalid
after calling regfree() on the regex out[] was taken against?  I do
not think so, and am confused by the comment.  After all, if we can
free only after copying the offsets out of "out", the posted patch
in question is already wrong ;-)

> Anyway, the ret approach taken here is fine.

Yup.

> "dir" is still leaking, by the way.  Probably worth a separate patch.

Good eyes.  But I'd actually say UNLEAK() is fine for that one.

Thanks.