bugGNU Octave - Bugs: bug #61839, fputs() + fdisp() do not use the...

 
 

bug #61839: fputs() + fdisp() do not use the fopen() character encoding

Submitter:  qx1147 <qx1147>
Submitted:  Mon 17 Jan 2022 11:31:58 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  qx1147 Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 05 Apr 2022 10:42:01 PM UTC, comment #5: 

No comments in a month, so I'm going to assume the patch worked.  Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Mon 07 Mar 2022 05:30:56 PM UTC, comment #4: 

I added an encoding facet for STL streams here and wrapped the output stream (of fopen) with it:
https://hg.savannah.gnu.org/hgweb/octave/rev/0826c503f294

That allowed to simplify a lot of code that was handling the output encoding in different places.

Maybe we could do something similar for the input stream.

This was changed on the default branch which will eventually be released as Octave 8 probably early 2023. Until then, users would need to rely on the workarounds mentioned earlier.

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Mon 28 Feb 2022 08:38:00 AM UTC, comment #3: 

Thank you for the effort.

qx1147 <qx1147>
Sun 27 Feb 2022 04:25:20 PM UTC, comment #2: 

This was pretty forward to implement for `fputs`. I pushed a change to the default branch here:
https://hg.savannah.gnu.org/hgweb/octave/rev/76398dfe2d55


However, it will be surprisingly complex to get this right for `fdisp`. IIUC, that function comes down to a call of the `print_raw` function of all octave_base_value types (in Octave core or any user code deriving directly or indirectly from octave_base_value). That's a lot.
Even if we would limit the change to character matrices in a first step, I don't see atm how we could implement this without a lot of refactoring. Ultimately, character matrices are printed here (for `fdisp`):
https://hg.savannah.gnu.org/hgweb/octave/file/ba07f81c8480/libinterp/corefcn/pr-output.cc#l2623

void
octave_print_internal (std::ostream& os, const charMatrix& chm,
                       bool pr_as_read_syntax,
                       int /* FIXME: extra_indent */,
                       bool pr_as_string)
{
  if (pr_as_string)
    {
      octave_idx_type nstr = chm.rows ();

      if (pr_as_read_syntax && nstr > 1)
        os << "[ ";

      if (nstr != 0)
        {
          for (octave_idx_type i = 0; i < nstr; i++)
            {
              octave_quit ();

              std::string row = chm.row_as_string (i);

              if (pr_as_read_syntax)
                {
                  os << '"' << octave::undo_string_escapes (row) << '"';

                  if (i < nstr - 1)
                    os << "; ";
                }
              else
                {
                  os << row;

                  if (i < nstr - 1)
                    os << "\n";
                }
            }
        }

      if (pr_as_read_syntax && nstr > 1)
        os << " ]";
    }
  else
    {
      os << "sorry, printing char matrices not implemented yet\n";
    }
}


But at that point, we already went through a lot of indirections. At that point in the code, it'll be difficult how to get the encoding that was set when "fopen"ing the file.

Even if we knew that encoding here, it would probably not be enough to convert only `row` to the output encoding. That might be enough for "well-behaved" encodings which implement ASCII characters at the same codepoint. But for "exotic" encodings, every part of the string would need to be converted.

Like already mentioned, that would need to be done for all octave_base_value classes.

At that point, it might be better to replace the `std::ostream` with something that does the conversion "on-the-fly". I experimented with `std::wbuffer_convert` and `std::codecvt_byname` (see attached diff). But the latter seems to fail for virtually any encoding I tested with:

terminate called after throwing an instance of 'std::runtime_error'
  what():  locale::facet::_S_create_c_locale name not valid


Maybe, we'd need to create our own encoding Facet on top of "uniconv"?



(file #52940)

Markus Mützel <mmuetzel>
Group administrator
Mon 17 Jan 2022 02:04:12 PM UTC, comment #1: 

Thanks for reporting.

I haven't checked. But it's very well possible that `fputs` and `fdisp` use different code paths (from `fprintf`) that don't respect the encoding setting yet.

I'll try to look into this when I come around to it.

Like you've already written, a possible workaround until this is fixed is to use:

fprintf(fid, "%s", str);


(It might be a good idea to use this "basic" format string to avoid surprises in case `str` happens to contain parts that look like format strings or escape sequences...)

Markus Mützel <mmuetzel>
Group administrator
Mon 17 Jan 2022 11:31:58 AM UTC, original submission:  

Different to fprintf(), fputs() + fdisp() do not use the fopen() character encoding (or the editor's default encoding as set in the Octave's editor preferences). Instead, these functions pass on the internal UTF-8 encoding (at least with Windows, did not test with Linux).
The following code:

str = '20°C';
fid = fopen('dummy.txt', 'wt', 'n', 'ISO-8859-1');
fprintf(fid,'%s\n', str);
fdisp(fid,str);
fputs(fid,str);
fclose(fid);


Produces the following text file (ISO-encoded):

20°C
20°C
20°C


The two last lines interpreted as UTF-8 (the first line is no valid UTF-8):

20°C
20°C


Note that for fprintf(), it does not matter whether the string is formatted or passed directly, i.e., fprintf(fid,str) also uses the correct encoding.

[Win10-21H2, Octave 6.2 or 7.0.90]

qx1147 <qx1147>

 

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

Attach Files:
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by qx1147 (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 group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-04-05 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2022-03-07 mmuetzel StatusConfirmed Ready For Test
    2022-02-27 mmuetzel Attached File- Added bug61839-fdisp-encoding.patch, #52940
        StatusNone Confirmed
        Operating SystemMicrosoft Windows Any
    2022-01-17 mmuetzel Release7.0.90 dev

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code