bugGNU Octave - Bugs: bug #41965, fwrite casts negative char values...

 
 

bug #41965: fwrite casts negative char values into zero

Submitter:  None
Submitted:  Wed 26 Mar 2014 09:53:21 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  mtmiller
Originator Name:  Alan W. Irwin Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * dev
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 08 Apr 2014 03:23:57 PM UTC, comment #7: 

The description of the problem here is wrong. It's not really about the signedness of char, but it was about a bug in fwrite. I've changed the description accordingly.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Thu 03 Apr 2014 11:58:44 PM UTC, comment #6: 

Actually, I think the problem with fwrite was actually a bug in fwrite, not that it couldn't or shouldn't work perfectly fine in this instance.  I checked in the following change for it on stable:

  http://hg.savannah.gnu.org/hgweb/octave/rev/aa861a98d84d

John W. Eaton <jwe>
Group administrator
Thu 03 Apr 2014 07:24:25 PM UTC, comment #5: 

Thanks for the comments jwe.

You're right, fputs would be better for this case. The fprintf (fid, "%s", str) usage seems to be used in at least a handful of functions under scripts, so perhaps these should be changed together as a cleanup changeset.

Then there are an additional handful of uses of fprintf (fid, "%s\n", str), that might also be more efficiently done as fputs (fid, [str "\n"]).

Mike Miller <mtmiller>
Group Member
Thu 03 Apr 2014 06:39:45 PM UTC, comment #4: 

Sorry to come to this discussion late.

Instead of fprintf (fid, "%s", str), I'd probably use fputs (fid, str) when there is no other data type conversion required.on.

Either way, fputs or fprintf are not exactly the same as fwrite becuase fwrite will pass ASCII 0, but fputs and fprintf won't.  I don't think that matters in this instance, but I'm not sure.

John W. Eaton <jwe>
Group administrator
Fri 28 Mar 2014 03:44:26 AM UTC, comment #3: 

Applied this change to the stable branch.

  http://hg.savannah.gnu.org/hgweb/octave/rev/2633b5f3106a

There should be no noticeable changes with the current Octave docstrings, but help and related functions should now support UTF-8 characters in Texinfo docstrings.

Mike Miller <mtmiller>
Group Member
Thu 27 Mar 2014 01:30:30 PM UTC, comment #2: 

In writing to the continuing thread on the mailing list, I just realized an even better solution for this that is not a workaround, simply use fprintf when dealing with human-readable text, both for consistency and to let the appropriate library routine deal with the conversion.

So the diff should be


-    fwrite (fid, text);
+    fprintf (fid, "%s", text);


I'll test this later and commit (away from dev box at the moment, if someone else is interested feel free to take care of this). This seems safe enough that it should work on the stable branch.

Mike Miller <mtmiller>
Group Member
Thu 27 Mar 2014 02:14:48 AM UTC, comment #1: 

Confirmed already in the discussion on the mailing list. Also affects the development version. Thanks for transcribing all of this detail to the bug report.

Although, for this particular example, keep in mind that technically the preferred texinfo formatting would be:


## -*- texinfo -*-
## @deftypefn  {Function File} {@var{a} =} fn (@var{x}, @dots{})
## The unicode character, @geq{}, is output
## @end deftypefn


And then it's up to the texinfo program to convert the macros to the output format.

But yes, in general for other non-ASCII characters that would be useful to us, like °, ×, µ, or even é this bug report still holds.

Mike Miller <mtmiller>
Group Member
Wed 26 Mar 2014 09:53:21 PM UTC, original submission:  

The _makeinfo_ function writes documentation strings to a temporary file using 

fwrite (fid, text);

That function assumes the documentation text is contained in an array of unsigned char's by default.  That's fine for documentation written in 7-bit ascii, but when the text contains 8-bit utf-8 characters, those 8-bit bytes are replaced by null characters as can be seen by the following simple example:


function test_fwrite_utf8
fid = fopen("test.out", "w")
  fwrite(fid, "The unicode character, ≥, is outputn")
endfunction


The net result of running that function is


irwin@raven> od -a test.out
0000000   T   h   e  sp   u   n   i   c   o   d   e  sp   c   h   a   r
0000020   a   c   t   e   r   ,  sp nul nul nul   ,  sp   i   s  sp   o
0000040   u   t   p   u   t  nl
0000046


If an additional argument "schar" is used for fwrite, all is well and
the utf8 characters are written to the file without issues which appears to prove that Octave strings are generally represented as an array of signed char's (as opposed to an array of unsigned characters as assumed by the _makeinfo_ function.

Indeed if the following patch


--- __makeinfo__.m_original     2014-03-26 13:56:42.741106684 -0700
+++ __makeinfo__.m      2014-03-26 13:56:19.005546479 -0700
@@ -120,7 +120,7 @@
     if (fid < 0)
       error ("__makeinfo__: could not create temporary file");
     endif
-    fwrite (fid, text);
+    fwrite (fid, text, "schar");
     fclose (fid);

     ## Take action depending on output type


is applied, then the following code


## -*- texinfo -*-
## @deftypefn  {Function File} {@var{a} =} fn (@var{x}, …)
## The unicode character, ≥, is output
## @end deftypefn

function test_utf8
printf("The unicode character, ≥, is outputn")
endfunction


yields the following help result:


warning: function ./__makeinfo__.m shadows a core library function
octave:1> help test_utf8
`test_utf8' is a function from the file /home/irwin/test_octave/test_utf8.m

 -- Function File: A = fn (X, …)
     The unicode character, ≥, is output


Without the above patch, the help output is truncated just before
the utf-8 symbol for greater than or equal to.  This is expected because fwrite writes null characters in place of the utf8 bytes without the above patch to the _makeinfo_ function.


Anonymous

 

(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 jordigh (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by None (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
    2014-04-08 jordigh Summary_makeinfo_ assumes unsigned char for input text when it should be signed char fwrite casts negative char values into zero
    2014-03-28 mtmiller StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2014-03-27 mtmiller StatusConfirmed Patch Submitted
        Assigned toNone mtmiller
    2014-03-27 mtmiller StatusNone Confirmed
        Release3.6.2 dev

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code