Web lists-archives.com

Re: [PATCH 17/34] mailinfo: release strbuf on error return in handle_boundary()




On 31 August 2017 at 19:21, René Scharfe <l.s.r@xxxxxx> wrote:
> Am 30.08.2017 um 20:23 schrieb Martin Ågren:
>> On 30 August 2017 at 19:49, Rene Scharfe <l.s.r@xxxxxx> wrote:
>>> Signed-off-by: Rene Scharfe <l.s.r@xxxxxx>
>>> ---
>>>   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