[Octave-bug-tracker] [bug #54405] octave_idx_type index integer overflow math check doesn't work correctly

[Dan Sebald]

Follow-up Comment #2, bug #54405 (project octave):

A complicating factor is that this octave_idx_type's sign is often used to
carry information, such as in this fread() example:


    if (one_elt_size_spec)
      {
        // If NR == 0, Matlab returns [](0x0).

        // If NR > 0, the result will be a column vector with the given
        // number of rows.

        // If NR < 0, then we have Inf and the result will be a column
        // vector but we have to wait to see how big NR will be.

        if (nr == 0)
          nr = nc = 0;
        else
          nc = 1;
      }
    else
      {
        // Matlab returns [] even if there are two elements in the size
        // specification and one is nonzero.

        // If NC < 0 we have [NR, Inf] and we'll wait to decide how big NC
        // should be.

        if (nr == 0 || nc == 0)
          nr = nc = 0;
      }


Another variable for the Inf specification could be easily added, rather than
using the sign of an octave_idx_type for meaning.

Then there is the float mantissa issue alluded to in Bug #54100.  Note that
the 


  get_size (const Array<double>& size,
            octave_idx_type& nr, octave_idx_type& nc,
            bool& one_elt_size_spec, const std::string& who)


and


  static octave_idx_type
  get_size (double d, const std::string& who)


appear to be converting the index size to double value, hence the mantissa can
only hold 53 or 52 bits, unless


        retval = math::nint_big (d);


is doing something to account for larger mantissa.

The conversion to float and back and using the sign of octave_idx_type to mean
something seems extraneous, complicating matters.  Perhaps a check is missing
somewhere.

The size specs come from:


static octave_value
do_fread (octave::stream& os, const octave_value& size_arg,
          const octave_value& prec_arg, const octave_value& skip_arg,
          const octave_value& arch_arg, octave_idx_type& count)
{
  count = -1;

  Array<double> size = size_arg.xvector_value ("fread: invalid SIZE
specified");


Hence, the size starts out as a double, then is converted to octave_idx_type
for nr and nc, then those are converted to float for the get_size() routines,
then converted back to nr and nc octave_idx_type.  It might be better if as
soon as possible the size was converted to octave_idx_type, i.e., the
fractional parts of


octave:4> zeros(2.2,3.7)
ans =

   0   0   0
   0   0   0


fractional parts are dropped immediately, say a new


  Array<octave_idx_type> xsize_value (const char *fmt, ...) const;


routine.  Difficult to follow; just trying to simplify.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?54405>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/