changeset 28221:ebff357efd56

maint: merge stable to default.
author John W. Eaton <jwe@octave.org>
date Tue, 14 Apr 2020 22:55:33 -0400
parents 8619dc0d3749 (current diff) 6cccc3c82175 (diff)
children 1f07b80db239
files libinterp/corefcn/file-io.cc test/io.tst
diffstat 3 files changed, 222 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/file-io.cc	Tue Apr 14 11:10:14 2020 -0700
+++ b/libinterp/corefcn/file-io.cc	Tue Apr 14 22:55:33 2020 -0400
@@ -447,7 +447,7 @@
       else
 #endif
         {
-          FILE *fptr = octave::sys::fopen (fname.c_str (), mode.c_str ());
+          FILE *fptr = octave::sys::fopen (fname, mode);
 
           retval = octave_stdiostream::create (fname, fptr, md, flt_fmt,
                                                encoding);
--- a/liboctave/system/lo-sysdep.cc	Tue Apr 14 11:10:14 2020 -0700
+++ b/liboctave/system/lo-sysdep.cc	Tue Apr 14 22:55:33 2020 -0400
@@ -43,6 +43,7 @@
 #  include <wchar.h>
 
 #  include "lo-hash.h"
+#  include "filepos-wrappers.h"
 #  include "unwind-prot.h"
 #endif
 
@@ -149,13 +150,201 @@
       return true;
     }
 
+#if defined (OCTAVE_USE_WINDOWS_API)
+
+    static bool check_fseek_ftell_workaround_needed (bool set_nonbuffered_mode)
+    {
+      // To check whether the workaround is needed:
+      //
+      //   * Create a tmp file with LF line endings only.
+      //
+      //   * Open that file for reading in text mode.
+      //
+      //   * Read a line.
+      //
+      //   * Use ftello to record the position of the beginning of the
+      //     second line.
+      //
+      //   * Read and save the contents of the second line.
+      //
+      //   * Use fseeko to return to the saved position.
+      //
+      //   * Read the second line again and compare to the previously
+      //     saved text.
+      //
+      //   * If the lines are different, we need to set non-buffered
+      //     input mode for files opened in text mode.
+
+      std::string tmpname = sys::tempnam ("", "oct-");
+
+      if (tmpname.empty ())
+        {
+          (*current_liboctave_warning_handler)
+            ("fseek/ftell bug check failed (tmp name creation)!");
+          return false;
+        }
+
+      std::FILE *fptr = std::fopen (tmpname.c_str (), "wb");
+
+      if (! fptr)
+        {
+          (*current_liboctave_warning_handler)
+            ("fseek/ftell bug check failed (opening tmp file for writing)!");
+          return false;
+        }
+
+      fprintf (fptr, "%s", "foo\nbar\nbaz\n");
+
+      std::fclose (fptr);
+
+      fptr = std::fopen (tmpname.c_str (), "rt");
+
+      if (! fptr)
+        {
+          (*current_liboctave_warning_handler)
+            ("fseek/ftell bug check failed (opening tmp file for reading)!");
+          return false;
+        }
+
+      unwind_action act ([fptr, tmpname] () {
+                           std::fclose (fptr);
+                           sys::unlink (tmpname);
+                         });
+
+      if (set_nonbuffered_mode)
+        ::setvbuf (fptr, 0, _IONBF, 0);
+
+      while (true)
+        {
+          int c = fgetc (fptr);
+
+          if (c == EOF)
+            {
+              (*current_liboctave_warning_handler)
+                ("fseek/ftell bug check failed (skipping first line)!");
+              return false;
+            }
+
+          if (c == '\n')
+            break;
+        }
+
+      off_t pos = octave_ftello_wrapper (fptr);
+
+      char buf1[8];
+      int i = 0;
+      while (true)
+        {
+          int c = fgetc (fptr);
+
+          if (c == EOF)
+            {
+              (*current_liboctave_warning_handler)
+                ("fseek/ftell bug check failed (reading second line)!");
+              return false;
+            }
+
+          if (c == '\n')
+            break;
+
+          buf1[i++] = static_cast<char> (c);
+        }
+      buf1[i] = '\0';
+
+      octave_fseeko_wrapper (fptr, pos, SEEK_SET);
+
+      char buf2[8];
+      i = 0;
+      while (true)
+        {
+          int c = fgetc (fptr);
+
+          if (c == EOF)
+            {
+              (*current_liboctave_warning_handler)
+                ("fseek/ftell bug check failed (reading after repositioning)!");
+              return false;
+            }
+
+          if (c == '\n')
+            break;
+
+          buf2[i++] = static_cast<char> (c);
+        }
+      buf2[i] = '\0';
+
+      return strcmp (buf1, buf2);
+    }
+
+#endif
+
     std::FILE *
     fopen (const std::string& filename, const std::string& mode)
     {
 #if defined (OCTAVE_USE_WINDOWS_API)
+
       std::wstring wfilename = u8_to_wstring (filename);
       std::wstring wmode = u8_to_wstring (mode);
-      return _wfopen (wfilename.c_str (), wmode.c_str ());
+
+      std::FILE *fptr = _wfopen (wfilename.c_str (), wmode.c_str ());
+
+      static bool fseek_ftell_bug_workaround_needed = false;
+      static bool fseek_ftell_bug_checked = false;
+
+      if (! fseek_ftell_bug_checked && mode.find ('t') != std::string::npos)
+        {
+          // FIXME: Is the following workaround needed for all files
+          // opened in text mode, or only for files opened for reading?
+
+          // Try to avoid fseek/ftell bug on Windows systems by setting
+          // non-buffered input mode for files opened in text mode, but
+          // only if it appears that the workaround is needed.  See
+          // Octave bug #58055.
+
+          // To check whether the workaround is needed:
+          //
+          //   * Create a tmp file with LF line endings only.
+          //
+          //   * Open that file for reading in text mode.
+          //
+          //   * Read a line.
+          //
+          //   * Use ftello to record the position of the beginning of
+          //     the second line.
+          //
+          //   * Read and save the contents of the second line.
+          //
+          //   * Use fseeko to return to the saved position.
+          //
+          //   * Read the second line again and compare to the
+          //     previously saved text.
+          //
+          //   * If the lines are different, we need to set non-buffered
+          //     input mode for files opened in text mode.
+          //
+          //   * To verify that the workaround solves the problem,
+          //     repeat the above test with non-buffered input mode.  If
+          //     that fails, warn that there may be trouble with
+          //     ftell/fseek when reading files opened in text mode.
+
+          if (check_fseek_ftell_workaround_needed (false))
+            {
+              if (check_fseek_ftell_workaround_needed (true))
+                (*current_liboctave_warning_handler)
+                  ("fseek/ftell may fail for files opened in text mode");
+              else
+                fseek_ftell_bug_workaround_needed = true;
+            }
+
+          fseek_ftell_bug_checked = true;
+        }
+
+      if (fseek_ftell_bug_workaround_needed
+          && mode.find ('t') != std::string::npos)
+        ::setvbuf (fptr, 0, _IONBF, 0);
+
+      return fptr;
+
 #else
       return std::fopen (filename.c_str (), mode.c_str ());
 #endif
--- a/test/io.tst	Tue Apr 14 11:10:14 2020 -0700
+++ b/test/io.tst	Tue Apr 14 22:55:33 2020 -0400
@@ -922,3 +922,34 @@
 
 %!assert <*53148> (double (sprintf ("B\0B")), [66, 0, 66])
 %!assert <*53148> (sscanf ("B\0B 13", "B\0B %d"), 13)
+
+%!test <58055>
+%!  w_modes = {"wb", "wt"};
+%!  r_modes = {"rb", "rt"};
+%!  f_texts = {"foo\nbar\nbaz\n", "foo\rbar\rbaz\r", "foo\r\nbar\r\nbaz\r\n"};
+%!  for i = 1:numel (w_modes)
+%!    w_mode = w_modes{i};
+%!    for j = 1:numel (r_modes);
+%!      r_mode = r_modes{j};
+%!      for k = 1:numel (f_texts);
+%!        f_text = f_texts{k};
+%!        fname = tempname ()
+%!        fid = fopen (fname, w_mode);
+%!        unwind_protect
+%!          fprintf (fid, "%s", f_text);
+%!          fclose (fid);
+%!          fid = fopen (fname, r_mode)
+%!          fgetl (fid);
+%!          pos = ftell (fid);
+%!          buf1 = fgetl (fid);
+%!          fgetl (fid);
+%!          fseek (fid, pos, SEEK_SET);
+%!          buf2 = fgetl (fid);
+%!          assert (buf1, buf2);
+%!        unwind_protect_cleanup
+%!          fclose (fid);
+%!          unlink (fname);
+%!        end_unwind_protect
+%!      endfor
+%!    endfor
+%!  endfor