[Coin-discuss] Segmentation fault in CoinPackedMatrix

[Matthew Galati]

I agree. Just the fact that you'd have to scan is a strong enough 
argument. I guess I should have read the documentation before trying to 
parse the source :)

http://www-124.ibm.com/developerworks/opensource/coin/Doxygen/Coin/class_coin_packed_matrix.html#z143_3

Matt


> 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
>>
> 
> 
> _______________________________________________
> Coin-discuss mailing list
> Coin-discuss at www-124.ibm.com
> http://www-124.ibm.com/developerworks/oss/mailman/listinfo/coin-discuss