Re: [nft PATCH 4/4] segtree: Refactor ei_insert()

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Tue, Jan 28, 2020 at 01:23:12PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Jan 23, 2020 at 03:30:49PM +0100, Phil Sutter wrote:
[...]
> > +	if (!merge) {
> > +		errno = EEXIST;
> > +		return expr_binary_error(msgs, lei->expr, new->expr,
> > +					 "conflicting intervals specified");
> >  	}
> 
> Not your fault, but I think this check is actually useless given that
> the overlap check happens before (unless you consider to consolidate
> the insertion and the overlap checks in ei_insert).

That's interesting, indeed. What's more interesting is how
interval_cmp() works: I assumed it would just sort by start element when
in fact interval size is the prominent aspect. In practice, this means
my changes work only as long as all intervals are of equal or decreasing
size. Does it make sense to uphold this ordering scheme?

> > -	__ei_insert(tree, new);
> > +	/* caller sorted intervals, so rei is either equal to lei or NULL */
> > +	rei = ei_lookup(tree, new->right);
> > +	if (rei != lei) {
> 
> Isn't this always true? I mean rei != lei always stands true?

Not if the second interval is entirely contained within the first one,
something like { 10-20, 12-14 }.

Cheers, Phil