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?
Yes, setelem_adjust() always checks for if (!(prev->flags & EXPR_F_REMOVE))
Your patch looks correct to me.
> diff --git a/src/intervals.c b/src/intervals.c
> index 451bc4dd4dd45..c0077c06880ff 100644
> --- a/src/intervals.c
> +++ b/src/intervals.c
> @@ -265,14 +265,12 @@ static void remove_elem(struct expr *prev, struct set *set, struct expr *purge)
> {
> struct expr *clone;
>
> - if (!(prev->flags & EXPR_F_REMOVE)) {
> - if (prev->flags & EXPR_F_KERNEL) {
> - clone = expr_clone(prev);
> - list_move_tail(&clone->list, &purge->expressions);
> - } else {
> - list_del(&prev->list);
> - expr_free(prev);
> - }
> + if (prev->flags & EXPR_F_KERNEL) {
> + clone = expr_clone(prev);
> + list_move_tail(&clone->list, &purge->expressions);
> + } else {
> + list_del(&prev->list);
> + expr_free(prev);
> }
> }
>
> @@ -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)
> adjust_elem_left(set, prev, i, add, purge);
> } else 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)
> adjust_elem_right(set, prev, i, add, purge);
> } else 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)
> split_range(set, prev, i, add, purge);
> } else {
> return -1;
> @@ -417,8 +412,7 @@ static int setelem_delete(struct list_head *msgs, struct set *set,
>
> 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) {
> list_move_tail(&prev->list, &purge->expressions);
> list_del(&i->list);
> expr_free(i);
