Re: non-ascii characters. I have cross-compiled it, but have no way to try it out.

[John Darrington]

For some reason my comments to this patch got chopped.  What I meant to say was:

This patch provides most of what is necessary to fix the issue on Windows which 
a number of users have complained about, viz:  not being able to read or write
files which contain non-ascii characters in the filenames or their path.

I have checked that it cross compiles for windows, but have no means of testing 
it.
Perhaps Harry can check that out.  I expect that it will have (re)introduced 
another
issue however:  It will break when trying to overwrite a file which already 
exists.

To fix that we need to have a special version of _wrename, either in PSPP or in 
Gnulib
which behaves in a more POSIX like manner.

Any other comments to this patch are welcome.

J'

On Sat, Oct 17, 2015 at 09:36:13PM +0200, John Darrington wrote:
     One thing which I expect still to fail, is overwriting files which already 
exist.
     To fix that we need to reimplement microsoft's buggy version of _wrename.
     
     Thanks.
     
     J'
     
     >From f5e3b6edda49aac07668de4918e6c76a3c35b1f9 Mon Sep 17 00:00:00 2001
     From: John Darrington <address@hidden>
     Date: Sat, 17 Oct 2015 17:29:31 +0200
     Subject: [PATCH] make-file.c: Behave better under windows operating systems
     
     Under w32 it is necessary to convert all file names to UTF-16 and prefix 
all
     the file related system calls with "_w".  Otherwise, calls involving 
filenames
     which contain non-ascii characters will not work properly.
     ---
      src/data/make-file.c |   93 
++++++++++++++++++++++++++++++++++++++------------
      1 file changed, 71 insertions(+), 22 deletions(-)
     
     diff --git a/src/data/make-file.c b/src/data/make-file.c
     index b5d6d58..7606d89 100644
     --- a/src/data/make-file.c
     +++ b/src/data/make-file.c
     @@ -41,11 +41,51 @@
      #include "gettext.h"
      #define _(msgid) gettext (msgid)
      
     +
     +#if defined _WIN32 || defined __WIN32__
     +#define WIN32_LEAN_AND_MEAN  /* avoid including junk */
     +#define UNICODE 1
     +#include <windows.h>
     +#define TS_stat _stat
     +#define Trename _wrename
     +#define Tunlink _wunlink
     +#define Topen _wopen
     +#define Tstat _wstat
     +
     +static TCHAR * 
     +convert_to_filename_encoding (const char *s, size_t len, const char 
*current_encoding)
     +{
     +  return (TCHAR *) recode_string ("UTF-16LE", current_encoding, s, len);
     +}
     +
     +
     +#else
     +typedef char TCHAR;
     +#define TS_stat stat
     +#define Trename rename
     +#define Tunlink unlink
     +#define Topen open
     +#define Tstat stat
     +
     +static TCHAR * 
     +convert_to_filename_encoding (const char *s, size_t len UNUSED, const 
char *current_encoding UNUSED)
     +{
     +  /* Non-windows systems don't care about the encoding.  
     +     The string is copied here, to be consistent with the w32 case.  */
     +  return xstrdup (s);
     +}
     +
     +#endif
     +
     +
      struct replace_file
        {
          struct ll ll;
     -    char *file_name;
     -    char *tmp_name;
     +    TCHAR *file_name;
     +    TCHAR *tmp_name;
     +
     +    char *tmp_name_verbatim;
     +    const char *file_name_verbatim;
        };
       
      static struct ll_list all_files = LL_INITIALIZER (all_files);
     @@ -58,24 +98,27 @@ replace_file_start (const struct file_handle *fh, 
const char *mode,
                          mode_t permissions, FILE **fp)
      {
        static bool registered;
     -  struct stat s;
     +  struct TS_stat s;
        struct replace_file *rf;
        int fd;
        int saved_errno = errno;
      
        const char *file_name = fh_get_file_name (fh);
      
     +  TCHAR * Tfile_name = convert_to_filename_encoding (file_name, strlen 
(file_name), fh_get_file_name_encoding (fh));
     +
        /* If FILE_NAME represents a special file, write to it directly
           instead of trying to replace it. */
     -  if (stat (file_name, &s) == 0 && !S_ISREG (s.st_mode))
     +  if (Tstat (Tfile_name, &s) == 0 && !S_ISREG (s.st_mode))
          {
            /* Open file descriptor. */
     -      fd = open (file_name, O_WRONLY);
     +      fd = Topen (Tfile_name, O_WRONLY);
            if (fd < 0)
              {
          saved_errno = errno;     
                msg (ME, _("Opening %s for writing: %s."),
                     file_name, strerror (saved_errno));
     +    free (Tfile_name);
                return NULL;
              }
      
     @@ -87,12 +130,13 @@ replace_file_start (const struct file_handle *fh, 
const char *mode,
          msg (ME, _("Opening stream for %s: %s."),
                     file_name, strerror (saved_errno));
                close (fd);
     +    free (Tfile_name);
                return NULL;
              }
      
     -      rf = xmalloc (sizeof *rf);
     +      rf = xzalloc (sizeof *rf);
            rf->file_name = NULL;
     -      rf->tmp_name = xstrdup (file_name);
     +      rf->tmp_name = Tfile_name;
            return rf;
          }
      
     @@ -103,32 +147,36 @@ replace_file_start (const struct file_handle *fh, 
const char *mode,
          }
        block_fatal_signals ();
      
     -  rf = xmalloc (sizeof *rf);
     -  rf->file_name = xstrdup (file_name);
     +  rf = xzalloc (sizeof *rf);
     +  rf->file_name = Tfile_name;
     +  rf->file_name_verbatim = file_name;
     +
        for (;;)
          {
            /* Generate unique temporary file name. */
     -      rf->tmp_name = xasprintf ("%s.tmpXXXXXX", file_name);
     -      if (gen_tempname (rf->tmp_name, 0, 0600, GT_NOCREATE) < 0)
     +      free (rf->tmp_name_verbatim);
     +      rf->tmp_name_verbatim = xasprintf ("%s.tmpXXXXXX", file_name);
     +      if (gen_tempname (rf->tmp_name_verbatim, 0, 0600, GT_NOCREATE) < 0)
              {
          saved_errno = errno;
                msg (ME, _("Creating temporary file to replace %s: %s."),
     -               rf->file_name, strerror (saved_errno));
     +               file_name, strerror (saved_errno));
                goto error;
              }
      
     +      rf->tmp_name = convert_to_filename_encoding (rf->tmp_name_verbatim, 
strlen (rf->tmp_name_verbatim), fh_get_file_name_encoding (fh));
     +
            /* Create file by that name. */
     -      fd = open (rf->tmp_name, O_WRONLY | O_CREAT | O_EXCL | O_BINARY, 
permissions);
     +      fd = Topen (rf->tmp_name, O_WRONLY | O_CREAT | O_EXCL | O_BINARY, 
permissions);
            if (fd >= 0)
              break;
            if (errno != EEXIST)
              {
          saved_errno = errno;
                msg (ME, _("Creating temporary file %s: %s."),
     -               rf->tmp_name, strerror (saved_errno));
     +               rf->tmp_name_verbatim, strerror (saved_errno));
                goto error;
              }
     -      free (rf->tmp_name);
          }
      
      
     @@ -138,9 +186,9 @@ replace_file_start (const struct file_handle *fh, 
const char *mode,
          {
            saved_errno = errno;
            msg (ME, _("Opening stream for temporary file %s: %s."),
     -           rf->tmp_name, strerror (saved_errno));
     +           rf->tmp_name_verbatim, strerror (saved_errno));
            close (fd);
     -      unlink (rf->tmp_name);
     +      Tunlink (rf->tmp_name);
            goto error;
          }
      
     @@ -168,14 +216,14 @@ replace_file_commit (struct replace_file *rf)
            int save_errno;
      
            block_fatal_signals ();
     -      ok = rename (rf->tmp_name, rf->file_name) == 0;
     +      ok = Trename (rf->tmp_name, rf->file_name) == 0;
            save_errno = errno;
            ll_remove (&rf->ll);
            unblock_fatal_signals ();
      
            if (!ok)
              msg (ME, _("Replacing %s by %s: %s."),
     -             rf->tmp_name, rf->file_name, strerror (save_errno));
     +             rf->tmp_name_verbatim, rf->file_name_verbatim, strerror 
(save_errno));
          }
        else
          {
     @@ -196,13 +244,13 @@ replace_file_abort (struct replace_file *rf)
            int save_errno;
      
            block_fatal_signals ();
     -      ok = unlink (rf->tmp_name) == 0;
     +      ok = Tunlink (rf->tmp_name) == 0;
            save_errno = errno;
            ll_remove (&rf->ll);
            unblock_fatal_signals ();
      
            if (!ok)
     -        msg (ME, _("Removing %s: %s."), rf->tmp_name, strerror 
(save_errno));
     +        msg (ME, _("Removing %s: %s."), rf->tmp_name_verbatim, strerror 
(save_errno));
          }
        else
          {
     @@ -218,6 +266,7 @@ free_replace_file (struct replace_file *rf)
      {
        free (rf->file_name);
        free (rf->tmp_name);
     +  free (rf->tmp_name_verbatim);
        free (rf);
      }
      
     @@ -231,7 +280,7 @@ unlink_replace_files (void)
          {
            /* We don't free_replace_file(RF) because calling free is unsafe
               from an asynchronous signal handler. */
     -      unlink (rf->tmp_name);
     +      Tunlink (rf->tmp_name);
          }
        unblock_fatal_signals ();
      }
     -- 
     1.7.10.4
     
     

-- 
Avoid eavesdropping.  Send strong encryted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.

signature.asc
Description: Digital signature