Re: octave patch

[David Bateman]

John W. Eaton wrote:
> On 25-Jan-2008, Jaroslav Hajek wrote:
> 
> | hello all developers,
> | I have noticed that while Octave handles Ranges (l:s:u) as special
> | objects in a smart way,
> | they are still being converted to integer vectors whenever used for 
> indexing.
> | The behaviour can easily be observed using a System Monitor which shows 
> memory:
> | do
> | n = 4e7; a = ones(n,1); b = ones(n,1);
> | now, the sysmon shows that a statement
> | a(1:n) = b;
> | allocates several mbytes of memory, while
> | a(:) = b;
> | does not.
> | 
> | The problem seems to be that idx_vector is not specialized to hold an
> | integer range (just single colon).
> | I have modified the idx_vector_rep class so to be specialized for
> | ranges (i.e. instead of data array,
> | store only base and step). The attached two patches (made with diff
> | -cp as advised in Octave's manual)
> | show the changes against the "Development" sources from
> | ftp://ftp.octave.org/pub/octave/bleeding-edge/octave-3.0.0.tar.bz2
> | Compiled and "make check"ed, all tests pass normally (only my normal
> | 12 fails with test-sparse occur, caused probably
> | by miscompilation of some of the UMFPACK et al. sparse libraries...]
> | 
> | The redundant allocation problem described above disappears.
> | 
> | I don't know about any octave benchmarks, but there might (hopefully)
> | be some positive performance impact.
> 
> I applied this patch and checked it in with the following ChangeLog
> entry.  In the future, it would help if you included the ChangeLog
> entry along with your patch.
> 
> I also added explicit initialization in all the constructors for the
> new range_base and range_step data members and fixed some formatting
> to match the conventions used in the rest of Octave.  Do you have your
> editor set to interpret TAB characters as being two spaces wide?  If
> so, I'd recommend not doing that for code you send to others, as they
> are not likely to use the same settings.
> 
> Thanks,
> 
> jwe
> 

I seem to get a seg-fault on start-up with this patch. The first error
from valgrind is

==31994==
==31994== Use of uninitialised value of size 8
==31994==    at 0x5CAAF8C: Array<char>::index(idx_vector&, int, char
const&) const (dim-vector.h:175)
==31994==    by 0x52FC06D:
octave_char_matrix_str::do_index_op_internal(octave_value_list const&,
bool, char) (ArrayN.h:121)
==31994==    by 0x5302601:
octave_char_matrix_str::do_index_op(octave_value_list const&, bool)
(ov-str-mat.h:91)
==31994==    by 0x52D2937:
octave_base_matrix<charNDArray>::subsref(std::string const&,
std::list<octave_value_list, std::allocator<octave_value_list> > const&)
 (ov-base-mat.cc:47)
==31994==    by 0x5346B7D: octave_value::subsref(std::string const&,
std::list<octave_value_list, std::allocator<octave_value_list> > const&,
int) (ov.cc:931)
==31994==    by 0x5495001: tree_index_expression::rvalue(int)
(pt-idx.cc:385)
==31994==    by 0x5496FF9: tree_index_expression::rvalue() (pt-idx.cc:396)
==31994==    by 0x5477BCB:
tree_argument_list::convert_to_const_vector(octave_value const*)
(pt-arg-list.cc:182)
==31994==    by 0x5290197:
symbol_table::fcn_info::fcn_info_rep::find(tree_argument_list*,
string_vector const&, octave_value_list&, bool&, int) (symtab.cc:448)
==31994==    by 0x5291526:
symbol_table::fcn_info::find(tree_argument_list*, string_vector const&,
octave_value_list&, bool&, int) (symtab.cc:652)
==31994==    by 0x5291764: symbol_table::do_find(std::string const&,
tree_argument_list*, string_vector const&, octave_value_list&, bool&,
int, bool) (symtab.cc:720)
==31994==    by 0x5292044: symbol_table::find(std::string const&,
tree_argument_list*, string_vector const&, octave_value_list&, bool&,
int, bool) (symtab.cc:666)
==31994==

Running with "octave -f" prevents the error on startup. Though with a
few quick tests I was unable to regenerate the segfault. Removing the
patch with

$ cd liboctave
$ cvs diff -u -r 1.68 -r HEAD idx-vector.cc > patch
$ cvs diff -u -r 1.54 -r HEAD idx-vector.h >> patch
$ cat patch | patch -p0 -R

and rebuilding addresses the issue. Sorry I can't see what the issue is
at this point.

D.