Range checking in liboctave

I think that the operators () and elem methods for the liboctave array classes should range check their argument indices. We could define corresponding optimized methods that don’t do range checks.

It’s all there, but maybe a little cryptic when you start working on the codebase :sweat:

Range checks are done by the “checkelem(n)” access-methods, e.g.

Array<T>::checkelem(n)

 template <typename T>
 T&
 Array<T>::checkelem (octave_idx_type n)
 {
   // Do checks directly to avoid recomputing slice_len.
   if (n < 0)
     octave::err_invalid_index (n);
   if (n >= slice_len)
     octave::err_index_out_of_range (1, 1, n+1, slice_len, dimensions);
 
   return elem (n);
 }

The optimized (more and more direct) access methods are:

Array<T>::operator()(n) { return elem (n); }

Array<T>::elem(n) { make_unique (); return xelem (n); } where make_unique ensures that the accessed array memory is not shared which another shallow array copy.

Array<T>::xelem(n) { return slice_data[n]; } where slice_data is the bare data stored in the array. This is the most “bare metal” access method.

Does this help?

IMHO operator () should call checkelem instead of elem.

Thanks for your humble opinion :wink:

The whole Octave codebase has to be reviewed carefully to make such a change happen. IMHO, this is very unlikely to happen soon.

In some earlier versions of Octave the range checking in operator () could be enabled by a preprocessor flag. I don’t remember whether it was defined by default.

Right, the --enable-bounds-check option was removed from the configure script in 2017, prior to the release of version 4.4. It was never enabled by default, and the consensus was that it wasn’t useful enough to keep maintaining.

This was discussed here, and the option was removed here.

If you like the feature, it may be easy enough to edit your own copy of Octave to use checkelem by default, or you might want to look into static analysis instead.