Hello!

On Wed, Sep 16, 2009 at 09:24:33AM -0700, Matthew Dempsky wrote:

> On Wed, Sep 16, 2009 at 3:35 AM, Maxim Dounin <mdounin@mdounin.ru> wrote:
> > Patch looks correct for me.  It looks a bit fragile though,
> > probably we need a bit more bulletproof code here.
>
> Would you mind elaborating on what you think is fragile about it?
> E.g., how would you rather this bug be patched?

I personally prefer something that won't explode once "goto
failed;" will be reused somewhere after first ngx_resolver_free(r,
name->data). Just adding "name->data = NULL;" should be enough
(though it's redundant right now).

> > More generally - resolver known to leak, and probably requires
> > code audit.  It would be fine if you look into it.  I believe
> > Artem Bokhan will help with testing (cc'd as I'm not sure he is on
> > English list).
>
> Great. I'll spend some time looking at the rest of the code then.

BTW, it looks like CNAME case in the same function will also leak
as it sets rn->u.cname without freeing it first.

Probably we just need something like allocation pools as used in
other parts of nginx code. No idea why Igor wasn't used them in
resolver. Igor, could you please explain reasons?

Maxim Dounin