bugGNU Octave - Bugs: bug #62723, sscanf returns an error when there...

 
 

bug #62723: sscanf returns an error when there is no error

Submitter:  Dureisseix <dureisseix>
Submitted:  Fri 08 Jul 2022 03:59:21 PM UTC
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Ready For Test Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Tue 22 Nov 2022 04:38:51 PM UTC, comment #8: 

> Additionally, I stumbled across some probably older code (maybe from when `error` didn't throw an exception immediately). I moved the clean-up code to before throwing the exception here:
> https://hg.savannah.gnu.org/hgweb/octave/rev/2a12350f9410


This actually won't have any effect.  I've never been happy with this, but the octave_stream class has it's own error() function which is not the global error function that Octave uses elsewhere.  I made a comment about this at the top of oct-stream.cc a long time ago because I too was very confused about this.  The comment is:

// Programming Note: There are two very different error functions used
// in the stream code.  When invoked with "error (...)" the member
// function from stream or base_stream is called.  This
// function sets the error state on the stream AND returns control to
// the caller.  The caller must then return a value at the end of the
// function.  When invoked with "::error (...)" the exception-based
// error function from error.h is used.  This function will throw an
// exception and not return control to the caller.  BE CAREFUL and
// invoke the correct error function!

The change you made

+                        is.clear (is.rdstate () & (~std::ios::failbit));
                         error (who, "format failed to match");
-                        is.clear (is.rdstate () & (~std::ios::failbit));

is still probably a good idea because anyone who is familiar with Octave is likely to have the same confusion.  Putting the is.clear call first works whichever version of error() is called.

Rik <rik5>
Project Administrator
Tue 22 Nov 2022 11:53:47 AM UTC, comment #7: 

If I understand the standard documentation correctly, trying to read a character past the end of the stream sets the eofbit and the failbit:
https://en.cppreference.com/w/cpp/io/basic_istream/get

There is still one case where the failbit isn't reset that is triggered by the example in comment #6.
I pushed a patch here that will hopefully make this pass without breaking other use cases:
https://hg.savannah.gnu.org/hgweb/octave/rev/b39bf92a2364

I believe that follows along the approach you used for bug #63383.

Additionally, I stumbled across some probably older code (maybe from when `error` didn't throw an exception immediately). I moved the clean-up code to before throwing the exception here:
https://hg.savannah.gnu.org/hgweb/octave/rev/2a12350f9410

I hope that won't cause other regressions.

Marking as ready for test again.

Markus Mützel <mmuetzel>
Project Member
Mon 21 Nov 2022 09:42:20 PM UTC, comment #6: 

Sadly, I am re-opening this bug report on the development branch (Octave 9).  I had to make extensive changes to sscanf in order to make it more Matlab compatible and to resolve issues with bug #63383.  However, this now causes a failure for this test:

[line,count,errmsg,nextindex] = sscanf(' ,1 ',' %s ',1)

Rik <rik5>
Project Administrator
Wed 03 Aug 2022 05:01:37 PM UTC, comment #5: 

Good point.

I added that test case here:
https://hg.savannah.gnu.org/hgweb/octave/rev/e8a46f801825

Markus Mützel <mmuetzel>
Project Member
Tue 02 Aug 2022 05:03:39 PM UTC, comment #4: 

I verified the fix, and will close this bug report.

The only other thing is that it might be nice to have a BIST regression test for this using the compact test case

[val, count, errmsg, nextpos] = sscanf('p','%c')

Rik <rik5>
Project Administrator
Tue 02 Aug 2022 04:49:37 PM UTC, comment #3: 

I pushed a change to the stable branch here that should fix this issue:
https://hg.savannah.gnu.org/hgweb/octave/rev/bcd75c07c9e8

It should be part of Octave 7.3.0 (if it will be released).
Or - if you're on Windows - tomorrow's nightly build here: https://buildbot.octave.space/#/download

Marking as ready for test.

Markus Mützel <mmuetzel>
Project Member
Tue 02 Aug 2022 03:43:31 PM UTC, comment #2: 

Here is another example which has the same issue:

>> [val, count, errmsg, nextpos] = sscanf('p','%c')

val = p
count = 1
errmsg = sscanf: format failed to match
nextpos = 2

ptoshkov
Sat 09 Jul 2022 04:36:15 AM UTC, comment #1: 

Confirmed.  This is a regression that happened between versions 5.2.0 and 6.4.0.

Rik <rik5>
Project Administrator
Fri 08 Jul 2022 03:59:21 PM UTC, original submission:  

Following bug report #56396 (no error when there is one) but in the counter case, I noticed an error returned in version 7.1.0 on MacOS for the following minimum working example:

clear all
[line,count,errmsg,nextindex] = sscanf(' ,1 ',' %s ',1)
[line,count,errmsg,nextindex] = sscanf(' ,1 aa',' %s ',1)
[line,count,errmsg,nextindex] = sscanf(" ,1 "," %s ",1)
[line,count,errmsg,nextindex] = sscanf(" ,1 aa"," %s ",1)

the output is

line = ,1
count = 1
errmsg = sscanf: format failed to match
nextindex = 5
line = ,1
count = 1
errmsg =
nextindex = 5
line = ,1
count = 1
errmsg = sscanf: format failed to match
nextindex = 5
line = ,1
count = 1
errmsg =
nextindex = 5

Note the two error messages "sscanf: format failed to match" that seem strange.
For information the same example works correctly (void error message) on version 5.2.0 on a windows machine, and also with matlab R2022a.

Thanks in advance.

Dureisseix <dureisseix>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by ptoshkov (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by dureisseix (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only project members can vote.

     

     

     

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-11-22 mmuetzel StatusConfirmed Ready For Test
    2022-11-21 rik5 StatusFixed Confirmed
        Open/ClosedClosed Open
        Release7.1.0 dev
    2022-08-02 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2022-08-02 mmuetzel StatusConfirmed Ready For Test
    2022-07-09 rik5 Summarysscanf return an error when there is not sscanf returns an error when there is no error
    2022-07-09 rik5 Item GroupMissed Error or Warning Regression
        StatusNone Confirmed

    Back to the top


    Powered by Savane 3.9