Re: [Patch] natString.cc: Some fixes for bounds checking arithmetic.

[Dalibor Topic <robilad at kaffe dot org>]

Ralph Loader wrote:
> > I was wondering about a similar thing in kaffe, and it may be saner to 
> > write the array index argument checking code once in a final class in 
> > Java and/or a static inline method in C++/function in C and let the 
> > compiler do the inlining magic.
>
>
> Sounds good in theory.  A couple of issues to deal with in practice:
>
> - sometimes you have start and length, sometimes you have start and
> end.  The APIs aren't consistent.

there is no official 'array bounds checking api' by sun, so we are free 
to define a good one, i.e. whatever suits you best ;) Here is an 
extremely verbose implementation (may be suitable for testing the inliner ;)

private static final void checkIfLess(int left, int right, int type) {
	if (left >= right) {
		throwException(type);
	}
}

private static final void checkIfGreaterOrEqual(int left, int right, int 
type) throws IndexOutOfBoundsException {
	if (left < right) {
		throwException(type);
	}
}

private static final void throwException(int type) throws 
IndexOutOfBoundsException {
		switch (type) {
		case INDEX:
			throw new IndexOutOfBoundsException();
			break;
		case ARRAY_INDEX:
			throw new ArrayIndexOutOfBoundsException();
			break;
		case STRING_INDEX:
			throw new StringIndexOutOfBoundsException();
			break;
		default:
			throw new Error("Exception type is messed up");
		}
	}
}

public static final void checkStartOffset(int start_off, int 
array_length, int type) throws IndexOutOfBoundsException {
	checkIfGreaterOrEqual(start_off, 0);
	checkIfLess(start_off, array_length);
}

public static final void checkEndOffset(int end_off, int array_length, 
int type) throws IndexOutOfBoundsException {
	checkIfGreaterOrEqual(end_off, 0);
	checkIfLess(end_off, array_length);
}

public static final void checkBounds(int start_off, int end_off, int 
array_length, int type) throws IndexOutOfBoundsException {
	checkStartOffset(start_off, array_length);
	checkEndOffset(end_off, array_length);
	checkGreaterOrEqual(end_off, start_off);
}

I could also imagine a set of bool testBounds(...) functions that don't 
throw an exception, but allow the caller to set up a more elaborate 
exception message. They could be used by the check* methods to simplify 
the design:

public static final void checkBounds(int start_off, int end_off, int 
array_length, int type) throws IndexOutOfBoundsException {
	checkStartOffset(start_off, array_length);
	checkEndOffset(end_off, array_length);
	if (testLess(end_off, start_off) {
		throwException(type, "End offset " + end_off + " is less than start 
offset " + start_off);
	}
}

with a throwException(int type, String message) method, and the check* 
methods chnaged in the obvious ways to call test* methods.

what do you think?

cheers,
dalibor topic