Re: [9fans] replica bug

[erik quanstrom <quanstro@quanstro.net>]

excellent find.  i'd forgotten about this pointy thing.

the problem of course is that the avl library uses the return value directly
as an index into a 3 element array.  this all comes to tears if the compare
function can return something other than {-1, 0, 1}.

On Thu Mar  3 07:26:22 EST 2011, rod@hemiola.co.uk wrote:
> that value directly.  The avl code requires that it's comparison
> function returns -1, 0 or 1.  Strcmp is guaranteed only to return an
> integer whose sign reflects the comparison result.
[...]
>
> PS. Subsequently I wondered if this problem occured elsewhere in the
> system.

i ran into this problem with nupas.  i also fixed the problem in replica
last april.  i tried to submit it, also deleting the private copy of the
avl library.  that wasn't accepted.  though i never saw it, the worry was
that there were some subtle yet important differences.

> memcmp, but also from the difference between the two score types.  As
> it happens "memcmp" is "safe" for both 386 and arm, and I am guessing
> that the latter comparison rarely happens. I also note anyway that the
> avl tree is only created and used if the undocumented -m option is
> specified on the command line.  It seems that the chance of provoking
> the problem here is small...

that function is clearly wrong, and worth fixing.  there's no reason to
leave a land mine around to trip on one day.  if the code is truely never
going to be used, then it should be removed.  otherwise, let's send in
the bomb disposal unit.  here's what i have for both in reverse order.

static int
scoretreecmp(Avl *va, Avl *vb)
{
	ScoreTree *a, *b;
	int i;

	a = (ScoreTree*)va;
	b = (ScoreTree*)vb;

	i = memcmp(a->score, b->score, VtScoreSize);
	if(i == 0)
		i = a->type - b->type;
	if(i > 0)
		i = 1;
	if(i < 0)
		i = -1;
	return i;
}

static int
entrycmp(Avl *a, Avl *b)
{
	int n;
	Entry *ea, *eb;

	ea = (Entry*)a;
	eb = (Entry*)b;

	n = strcmp(ea->name, eb->name);
	if(n > 0)
		n = 1;
	else if(n < 0)
		n = -1;
	return n;
}

- erik