bugGNU Octave - Bugs: bug #58617, cell2struct silently truncates...

 
 

bug #58617: cell2struct silently truncates char matrices

Submitted by:  Kai Torben Ohlhus <siko1056>
Submitted on:  Fri 19 Jun 2020 02:31:55 AM UTC  
 
Category:  Interpreter Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Confirmed Assigned to:  Kai Torben Ohlhus <siko1056>
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Mon 22 Jun 2020 05:16:42 AM UTC, comment #4: 

Rik, thank you for going into this.  This surprised me indeed, too.

// compile with:
//
//  (clan)g++ -std=c++11 -Wall -Wextra -pedantic

#include <iostream>
#include <memory>
#include <vector>

std::unique_ptr<std::vector<int>> global_stuff;

std::vector<int> do_something () {
  std::vector<int> retval;

  return *global_stuff;

  return retval;
}

int main () {
  global_stuff = std::unique_ptr<std::vector<int>> (new std::vector<int> ({1, 2, 3}));

  std::vector<int> val = do_something ();

  for (auto const& v : val) {
    std::cout << v << ' ' << std::endl;
  }

  return 0;
}

Entirely undetected my garbage in both GCC and clang?!

Anyways, I fixed this
https://hg.savannah.gnu.org/hgweb/octave/rev/bc521cd24922

It might take more time to get into the rest, where to fix this appropriately.

Kai Torben Ohlhus <siko1056>
Project MemberIn charge of this item.
Fri 19 Jun 2020 08:14:04 PM UTC, comment #3: 

There are problems all the way down the line.  The cellstr_value() function that is getting called is in ov-cell.cc.  It contains two return statements which is grammatically incorrect for C++.  Only the first has any effect, and I don't know why there isn't a warning during compilation about this.

Array<std::string>
octave_cell::cellstr_value (void) const
{
  Array<std::string> retval;

  if (! iscellstr ())
    error ("invalid conversion from cell array to array of strings");

  if (cellstr_cache->isempty ())
    *cellstr_cache = matrix.cellstr_value ();

  return *cellstr_cache;

  return retval;
}

Rik <rik5>
Project Administrator
Fri 19 Jun 2020 07:49:15 PM UTC, comment #2: 

Taking a look at the code in ov-struct.cc for cell2struct I see

  if (! (args(1).iscellstr () || args(1).is_char_matrix ()))
    error ("cell2struct: FIELDS must be a cell array of strings or a character matrix");

  int dim = 0;

  if (nargin == 3)
    {
      if (! args(2).is_real_scalar ())
        error ("cell2struct: DIM must be a real scalar");

      dim = args(2).int_value () - 1;
    }

  if (dim < 0)
    error ("cell2struct: DIM must be a valid dimension");

  const Cell vals = args(0).cell_value ();
  const Array<std::string> fields = args(1).cellstr_value ();

  octave_idx_type ext = (vals.ndims () > dim ? vals.dims ()(dim) : 1);

  if (ext != fields.numel ())
    error ("cell2struct: number of FIELDS does not match dimension");

So, the problem is a little deeper.  Have to look at cellstr_value() and see how it operates.

Rik <rik5>
Project Administrator
Fri 19 Jun 2020 07:12:35 PM UTC, comment #1: 

This is most likely because string_value() member function of octave_value class takes just the first row from a char array().

Long term fix is probably to fix the way this internal function works.  See bug #49536 reported by Mike Miller.

As an aside, if you make this a cellstr then you get an error message which makes sense

octave:3> cell2struct({[1,2]}, cellstr (['foo';'bar';'baz']))
error: cell2struct: number of FIELDS does not match dimension

Rik <rik5>
Project Administrator
Fri 19 Jun 2020 02:31:55 AM UTC, original submission:  

A subtle truncation happens when calling

containers.Map(['foo';'bar';'baz'], [1,2])

https://lists.gnu.org/archive/html/octave-maintainers/2020-06/msg00088.html

The cause is that cell2struct allows the following:

Octave 6.0.1:

>> cell2struct({[1,2]}, {['foo','bar','baz']})
ans =

  scalar structure containing the fields:

    foobarbaz =

       1   2


>> cell2struct({[1,2]}, {['foo';'bar';'baz']})
ans =

  scalar structure containing the fields:

    foo =

       1   2

while Matlab R2020b does not:

>> cell2struct({[1,2]}, {['foo','bar','baz']})

ans =

  struct with fields:

    foobarbaz: [1 2]

>> cell2struct({[1,2]}, {['foo';'bar';'baz']})
Error using cell2struct
Field name must be a character vector or string scalar.

I agree to Matlab's behavior in this case, because my valuable key is truncated, without a warning.

Kai Torben Ohlhus <siko1056>
Project MemberIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by siko1056 (Submitted the item)
  • -email is unavailable- added by siko1056 (Discovered the bug.)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-06-22 siko1056 Assigned toNone => siko1056
    2020-06-19 rik5 Dependencies- => Depends on bugs #49536
    2020-06-19 rik5 StatusNone => Confirmed
        Operating SystemBSD => Any
    2020-06-19 siko1056 Carbon-Copy- => Added abdallah elshamy <abdallah.k.elshamy@gmail.com>

    Back to the top


    Powered by Savane 3.5