#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 )
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 14 years ago by
Status: | new → assigned |
---|
comment:2 Changed 14 years ago by
Milestone: | later → 0.3 (Public release) |
---|
comment:3 Changed 14 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:7 Changed 14 years ago by
Replying to peter:
Why is there a assert on the passed vector at
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 follow-up: 9 Changed 14 years ago by
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 Changed 14 years ago by
Well, you might be right ... I'll change the implementation to become more robust. Have you tested your code sample?
(In [783]) Fixes #155. Added asserts to check vectors in all non-const functions since no inline functions exists anymore.