On 2022-09-21, at 13:48:27 +0200, Florian Westphal wrote:
> Jeremy Sowden <jeremy@xxxxxxxxxx> wrote:
> > @@ -619,24 +622,12 @@ void interval_map_decompose(struct expr *set)
> >
> > if (!mpz_cmp(i->value, expr_value(low)->value)) {
> > expr_free(i);
> > - i = low;
> > + compound_expr_add(set, low);
> > } else {
> > - i = range_expr_alloc(&low->location,
> > - expr_clone(expr_value(low)), i);
> > - i = set_elem_expr_alloc(&low->location, i);
> > - if (low->etype == EXPR_MAPPING) {
> > - i = mapping_expr_alloc(&i->location, i,
> > - expr_clone(low->right));
> > - interval_expr_copy(i->left, low->left);
> > - } else {
> > - interval_expr_copy(i, low);
> > - }
> > - i->flags |= EXPR_F_KERNEL;
> > -
> > + add_interval(set, low, i);
> > expr_free(low);
> > }
> >
> > - compound_expr_add(set, i);
>
> This results in a memory leak:
>
> __interceptor_malloc libsanitizer/asan/asan_malloc_linux.cpp:145
> xmalloc src/utils.c:36
> xzalloc src/utils.c:75
> expr_alloc src/expression.c:46
> constant_expr_alloc src/expression.c:420
> interval_map_decompose src/segtree.c:619
I did try running the new shell test under valgrind: lots of noise, not
a lot of signal. :)
> Before, 'i' was assigned to the compund expr, but thats no longer the
> case.
> Does this look good to you?
Yes, LTGM.
> If so, I will sqash this before applying:
>
> diff --git a/src/segtree.c b/src/segtree.c
> --- a/src/segtree.c
> +++ b/src/segtree.c
> @@ -621,13 +621,14 @@ void interval_map_decompose(struct expr *set)
> mpz_bitmask(i->value, i->len);
>
> if (!mpz_cmp(i->value, expr_value(low)->value)) {
> - expr_free(i);
> compound_expr_add(set, low);
> } else {
> add_interval(set, low, i);
> expr_free(low);
> }
>
> + expr_free(i);
> +
> out:
> if (catchall)
> compound_expr_add(set, catchall);
>
Thanks,
J.
Attachment:
signature.asc
Description: PGP signature
