Re: gplot.txt

[Daniel J Sebald]

On 03/04/2016 12:26 AM, Lachlan Andrew wrote:
> On 4 March 2016 at 16:37, Daniel J Sebald<address@hidden>  wrote:
>
> > assign memory even from the start--not a bad strategy.  However, at that
> > point, the member pointers r and d are initialized as r(0) and d(0).  And
> > 2)
> >        delete [] r;
> >
> > Now, the first line of code is more questionable.  Typically one isn't
> > supposed to delete a NULL pointer.
>
> Good catch, but deleting NULL pointers is valid in all
> standards-compliant compilers, and some people advocate that rather
> than checking for NULL first.  See the discussion at
>
> http://stackoverflow.com/questions/4190703/is-it-safe-to-delete-a-null-pointer#4190737

Yes, appears to be fine.

It turns out that ::change_length() is accessed much more than I 
thought.  It is used in the header in inline function 
Sparse::change_capacity (octave_idx_type nz), and that routine is used 
at least a half dozen times in the source file.

Anyway, so the path that "speye(10)" follows through the instantiation 
(Sparse::Sparse) is

  else if (a_scalar)
    {
[snip]
      else
        {
          idx_vector rr = r;
          idx_vector cc = c;
          const octave_idx_type *rd = rr.raw ();
          const octave_idx_type *cd = cc.raw ();

I did a deeper dive on the code in that path, and I too find it to be 
fine in terms of indexing.  It is a bit tricky to follow because the 
array length is one longer than the number of nc, but I can't find any 
place where an index (of index) might be one index out of bounds.

The one thing that has me a bit confused is this raw() function:

const octave_idx_type *
idx_vector::raw (void)
{
  if (rep->idx_class () != class_vector)
    *this = idx_vector (as_array (), extent (0));

  idx_vector_rep * r = dynamic_cast<idx_vector_rep *> (rep);

  assert (r != 0);

  return r->get_data ();
}

I verified that the reassignment of "this" takes place as a result of 
the rr.raw () and cc.raw() calls.  And here is the definition of idx_vector:

  // Directly pass extent, no checking.
  idx_vector (const Array<octave_idx_type>& inda, octave_idx_type ext)
    : rep (new idx_vector_rep (inda, ext, DIRECT))
  { }

All this does basically--from the perspective of within raw()--is do

  rep = new idx_vector_rep (inda, ext, DIRECT)

So, one question I have is what happens to the old/new contents of "rep" 
if a new "rep" is being assigned?  I printed out the value of rep coming 
into raw() and got something non-zero:

octave:1> speye(10);
rep = 33754304
rep = 33753072

So, accessing rep->idx_class () should be valid, no matter the compiler. 
 (idx_class() is a virtual function, so maybe even if rep=0 the call 
rep->idx_class() is fine).  It seems there is a new rep in a new hunk of 
memory created by calling the constructor idx_vector (,).  And then 
"*this =" simply means that the function takes on the identity of the 
newly created object casting the new object's "rep" to get its data. 
However, to me it seems like some type of new object was created that is 
lost track of somehow.  There is no type of delete within Sparse.c on 
the cc or dd pointers.

Lachlan, does that idx_vector::raw (void) function make sense to you?

Dan