Thread: Thoughts on adding safety checks to a library

I've been working on an array library and now I'm thinking of adding built-in protections for things like out-of-bounds array accesses and such. Currently there are some sanity checks in place but it really could be more robust.

One of the ideas I've been considering is to report the error in detail and then exit. That seems better in the long run because then you don't necessarily have to resort to debugging. Just read the read the error that's been logged and the fix will likely be obvious. On the other hand, most of the library is implemented as macros so that does impose some limitations on forming expressions that can both return a result *and* fail on error.

But then also what if the application is already properly bounds checked? It doesn't really make sense to waste so many precious CPU cycles in tight loops in such cases. So maybe it should just be an opt-in feature? I was also wondering if there are any good generic approaches to that end. Because I'm really not looking forward to putting together that potential mess of code!

Originally Posted by aghast

Don't worry about it.

Calling code should be bounds checked by correct construction, not by adding checks:

for (int i = 0; i < length(array); ++i)

This avoids the need for any kind of additional checking, and guarantees that your code will never find any problems.

So why would you add checks? In case the caller does something stupid and fails to construct valid accesses.

That's a programming error, and deserves to be caught and reported as soon as possible.

Therefore, you want to have the checks in the low-level code. You want them to do something drastic, like ending the program.

So, what about performance costs? Well, make them cheap! If you're comparing a number in one register with a number in another register, that's cheap. If the "length" of your array is a stored value (not dynamically computed each time), and you're continually re-loading it, then it's in cache and doesn't cost any cycles (except the first one).

Thus, you need to make sure that the checks you do are against stored values, and are fast in the no-error case. (In the error case, you're going to kill the program, so take all the time you want!)

I'd say something like:

#define array_at(ARR, IDX) ((ARR)->ar_data[(IDX) < (ARR)->ar_length ? (IDX) : array_bounds_error((ARR), (IDX))])

Thanks! You're right though, I should just embed it at the lowest level. But I probably will make it opt-in rather than opt-out. That way, the user can decide for themselves which way to go. Most will probably turn it on for debugging and off for release builds.