[Coin-discuss] Segmentation fault in CoinPackedMatrix

[Laszlo Ladanyi]

It was intentional that the append{Major,Minor}Vector() methods do not accept
out of range indices. Basically, if they do then if there is an error in the
input vector (the one to be appended) then the append routines would silently
"correct" it, i.e., expand the matrix. This is definitely not what you'd want,
since then bugs would show up a lot later in the run. Not to mention that
the argument vector is compressed, so if the matrix is to be expanded we
can't really know what should be the new minor dimension (Though it could
probably be set safely to the largest index in the argument vector.) So at the
time we decided that the vector to be appended is assumed to be of the same
size as the matrix and having out of range indices is an error. Not to mention
that this way the indices in the vector need not be scanned just to check
whether the matrix needs to be resized. The scanning is still there, but
ifdef'd with COIN_DEBUG.

We can change the behavior of the append methods, but I feel the arguments
above are strong enough to put the user through a bit of inconvenience by
requesting an explicit resize if the user really wants to append vectors
longer than the matrix. I believe the code is cleaner this way (including
the user's code, since the explicit resizing indicates the long
vector) and more efficient. What do you think?

--Laci

On Thu, 22 Jan 2004, Matthew Galati wrote:

> sorry... typo in last post
> 
> "That is, it does *not* handle the case where the minor... "
> 
> 
> Matthew Galati wrote:
> > Looking over appendMajorVector again, I noticed that resize is only 
> > called if the buffer for major dimension (cols) or number of elements is 
> > exceeded. That is, it does handle the case where the minor dimension is 
> > too small - hence, the throw statement. So, for using the append 
> > functions, you do need to setDimensions first. It would be nice to have 
> > a resize that can handle this. But, the current implementation makes 
> > sense (in most cases), since typically you append columns then rows in 
> > sequence. So, in each call to appendCol you are operating on a fixed 
> > size of rows, and the number of columns is incremented. Then, when you 
> > appendRow it has the right number of columns (which is again fixed). If 
> > you do have an application that might add both at the same time, you can 
> > call setDimensions to resize your matrix beforehand.
> > 
> > Matt
> > 
> > 
> > 
> >> Ok. I tried to add the COIN_DEBUG and it actually throws the exception 
> >> if the minor dimension is not set to 2. If I do a 
> >> check.setDimensions(2,0) then it works. As I read the code this must 
> >> always be done? As you note, the only reason for having this throw 
> >> block is a bug in the resize or that it is simply an invariant? And by 
> >> the way the small code snip that I did seems to dump only on Linux and 
> >> not MSV6 while the orginal code dumps on both. Obviously this was 
> >> without COIN_DEBUG defined. The fix for now is to call setDimensions 
> >> which is possible in my application since the number of rows is 
> >> constant. Thanks.
> >>
> >> 'Jesper
> > 
> > 
> 
> _______________________________________________
> Coin-discuss mailing list
> Coin-discuss at www-124.ibm.com
> http://www-124.ibm.com/developerworks/oss/mailman/listinfo/coin-discuss
>