Opened 15 years ago

Closed 15 years ago

Last modified 15 years ago

#155 closed enhancement (fixed)

Current vector implementation can cause "unexpected" runtime aborts if bad programs are created.

Reported by: Jari Häkkinen Owned by: Jari Häkkinen
Priority: major Milestone: yat 0.3 (Public release)
Component: utility Version: trunk
Keywords: Cc:

Description (last modified by Jari Häkkinen)

The summary is somewhat cryptic. The problem arises from the fact that vectors can be disguised const views. A const view vector will not initialize the internal v_ variable as expected for non const vectors. This will cause unexpected runtime behaviour when a const view vector is not declared as a const vector. A non-const (const view) vector will allow syntactical changes to the element values, i.e. compilation will be succesful, but the program will abort at runtime:

const vector vm(13,1.0);
vector v1(vm,2,4);       // bad code!
v1(0)=-123;              // accepted by compiler, runtime failure will occur
const vector v2(vm,2,4);
v2(0)=-123;              // not acceptable for the compiler

The problem of unexpected aborts can be made somewhat clearer for developers by adding assert(v_) in all non-const vector member functions. This is not implement since doing the assert change will conflict with the coding style: No asserts in .h files. The resolution could be to move all non-const inline member functions to the .cc file and remove the inlining.

Change History (10)

comment:1 Changed 15 years ago by Jari Häkkinen

Status: newassigned

comment:2 Changed 15 years ago by Jari Häkkinen

Milestone: later0.3 (Public release)

comment:3 Changed 15 years ago by Jari Häkkinen

Description: modified (diff)

comment:4 Changed 15 years ago by Jari Häkkinen

Resolution: fixed
Status: assignedclosed

(In [783]) Fixes #155. Added asserts to check vectors in all non-const functions since no inline functions exists anymore.

comment:5 Changed 15 years ago by Peter

Why is there a assert on the passed vector at

source:/trunk/yat/utility/vector.cc@257#783

comment:6 Changed 15 years ago by Peter

Sorry meant on line on line 257

comment:7 in reply to:  5 Changed 15 years ago by Jari Häkkinen

Replying to peter:

Why is there a assert on the passed vector at

source:/trunk/yat/utility/vector.cc@783#257

Impressive, comments after a ticket closure!

Well, the reason is that in functions like the one you reference it is not known whether the passed vector is a const vector before the call (of the referenced function). Obviously it is const inside this function but since we pass references we have to assume that it may be non-const before the call. The vector class is designed to fail badly if a vector that is a disguised const view into another vector (or matrix). It should be emphasised, if the programmer does his job properly this is not a problem, see the doxygen documentation for the vector class for an example of bad and proper use of const vector views.

So, why is this a problem? A long story in a very short version: C++ compilers have problems tracking constness of blobs pointed to by pointers. If you have a const pointer to a struct, you cannot change the struct but you can change the contents of the struct!

comment:8 Changed 15 years ago by Peter

I assume I am missing something here (and thus keep the ticket closed)

However as I understand the implementation the following code snippet will be asserted:

ifstream is("data");
const vector v(is);
const vector view(v,0,5,1);
vector v2(5,2.0);

v2.mul(view);

right??? or what am I missing here?

As I understand it view.v_ is equal to NULL when mul is called. Moreover, other is const within mul function so I assume underlying function call

int status=gsl_vector_mul(v_,other.v_);

does not change other.v_. So what is the problem?

Is the problem that there is no gsl_vector_mul for gsl_vector_views?

comment:9 in reply to:  8 Changed 15 years ago by Jari Häkkinen

Well, you might be right ... I'll change the implementation to become more robust. Have you tested your code sample?

comment:10 Changed 15 years ago by Jari Häkkinen

(In [784]) References #155. Removed potential null pointer usage. Some unwanted asserts removed.

Note: See TracTickets for help on using tickets.