On 31 August 2017 at 19:21, René Scharfe <l....@web.de> wrote:
> Am 30.08.2017 um 20:23 schrieb Martin Ågren:
>> On 30 August 2017 at 19:49, Rene Scharfe <l....@web.de> wrote:
>>> Signed-off-by: Rene Scharfe <l....@web.de>
>>> ---
>>> mailinfo.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/mailinfo.c b/mailinfo.c
>>> index b1f5159546..f2387a3267 100644
>>> --- a/mailinfo.c
>>> +++ b/mailinfo.c
>>> @@ -911,48 +911,49 @@ static int find_boundary(struct mailinfo *mi, struct
>>> strbuf *line)
>>> static int handle_boundary(struct mailinfo *mi, struct strbuf *line)
>>> {
>>> struct strbuf newline = STRBUF_INIT;
>>>
>>> strbuf_addch(&newline, '\n');
>>> again:
>>> if (line->len >= (*(mi->content_top))->len + 2 &&
>>> !memcmp(line->buf + (*(mi->content_top))->len, "--", 2)) {
>>> /* we hit an end boundary */
>>> /* pop the current boundary off the stack */
>>> strbuf_release(*(mi->content_top));
>>> FREE_AND_NULL(*(mi->content_top));
>>>
>>> /* technically won't happen as is_multipart_boundary()
>>> will fail first. But just in case..
>>> */
>>> if (--mi->content_top < mi->content) {
>>> error("Detected mismatched boundaries, can't
>>> recover");
>>> mi->input_error = -1;
>>> mi->content_top = mi->content;
>>> + strbuf_release(&newline);
>>> return 0;
>>> }
>>
>> Since this code path can't be taken (or so it says): How did you find
>> this and the others? Static analysis? Grepping around?
>
> Code inspection: I looked for functions with STRBUF_INIT that return
> without calling strbuf_release() with "git grep -W STRBUF_INIT" and
> searching for return in less(1).
Thanks for sharing.
I read through this series and thought it looked good. One thing I like
is that it doesn't just blindly apply some standard solution in all
patches, but always tries to do what it feels is "right" for each
particular function.
Martin
