On Wed, Apr 13, 2022 at 04:47:07PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Apr 13, 2022 at 04:38:02PM +0200, Phil Sutter wrote:
> > On Wed, Apr 13, 2022 at 04:27:07PM +0200, Pablo Neira Ayuso wrote:
> > [...]
> > > > The one with EXPR_F_REMOVE comes *after* the one to be removed, right?
> > >
> > > Right, the other way around actually.
> > >
> > > > My question again: Is it possible for 'prev' to have EXPR_F_REMOVE set?
> > > > Maybe I miss something, but to me it looks like not although the code
> > > > expects it.
> > >
> > > prev never has EXPR_F_REMOVE, so it points to an existing element.
> >
> > So below change should be fine?
>
> Wait.
>
> > diff --git a/src/intervals.c b/src/intervals.c
> > index 451bc4dd4dd45..c0077c06880ff 100644
> > --- a/src/intervals.c
> > +++ b/src/intervals.c
> [...]
> > @@ -360,18 +358,15 @@ static int setelem_adjust(struct set *set, struct expr *add, struct expr *purge,
> > {
> > if (mpz_cmp(prev_range->low, range->low) == 0 &&
> > mpz_cmp(prev_range->high, range->high) > 0) {
> > - if (!(prev->flags & EXPR_F_REMOVE) &&
> > - i->flags & EXPR_F_REMOVE)
> > + if (i->flags & EXPR_F_REMOVE)
>
> This chunk is not correct.
>
> User might ask to delete an element which does not exist.
>
> Then, you might find two consecutive EXPR_F_REMOVE.
>
> Only the initial chunk in this patch is fine.
Wait, you mean prev is always !EXPR_F_REMOVE, then this should be
fine.
