[ruby-core:24938] Re: regparse.c - patch to fix memory leak

At 12:30 15.08.2009, Nobuyoshi Nakada wrote:

>OK, thank you, I could confirm the problem and your patch seems
>to work fine. 

Great, thanks for the double-check!

>But it's difficult to make memory leak tests portable.

Despite the difficulty of cross-platform memory leak tests, fixing a leak on one platform should also benefit the other platforms.

>Only thing feels strange to me is:
>
>  r = parse_exp(&node, tok, term, src, end, env);
>  if (r < 0) {
>    onig_node_free(node);
>    return r;
>  }
>
>The node is allocated in parse_exp() regardless the error, and
>callers have to free it on the result.  In such case, shouln't
>these functions free nodes allocated inside them?

Looks like question of coding principle to me. It should not matter as long as the approach is consistent.

In Oniguruma I see that the calling function usually frees nodes on error. It would certainly be possible to free nodes inside the called function, but there are more such places in parse_exp() than that function itself is called (2 times only). Hence, from a coding perspective, freeing just twice to me looks simpler and less error prone than doing so multiple times inside the function.

Ralf