Re: [v2 PATCH] canonicalize-lgpl: Return file name using '/' consistently on MinGW.

[Jan Nieuwenhuizen]

Bruno Haible writes:

Hi!

> Jan Nieuwenhuizen wrote:
>> Yes, but this example shows the mixing of three problems:
>>   (1) wrong drive letter casing,
>>   (2) difference in type of slash,
>>   (3) difference in casing of the file name.

Please find attached a patch that only cares about (2), because the
seemingly arbitrarily returning of different flavours of '/' '\\' for
the same file name is simply a bug.

> I claim that
>
>   * Caring about (1), (2), (3) should be done in utility functions outside of
>     realpath(), since they can be useful even on relative file names.

Ah right.  If the need arises, I can imagine someone working on a patch
to factor out "slashify_file_name" or something for people who need this
behaviour on getcwd results, for example.

>   * Caring about (2) is helpful when passing file names to other programs via
>     the command line, namely:
>       - when you pass a file name to a Java-based program on Windows, you need
>         to convert '\\' to '/', in order to avoid interpretation of '\\t' and
>         such,

Nice, a third reason why we want this patch.

>       - when you pass a file name to other Windows programs, you need
>         to convert '/' to '\\', in order to avoid interpretation of 
> '/something'
>         as a command-line option.

Hmm.  If you do this, then you already have Windows-specific code, which
should be *really* exceptional for GNU, right?  Also, the client code
must already be taking care of (2), otherwise it would not work right
now.  Fixing canonicalize_file_name has no impact, even for this
exceptional case, so this is a "neutral".

But this patch does help to avoid having to *always* write
windows-specific code in GNU software when it is compiled for Windows.

Interesting how you failed to miss two other reason to care about
(2)

        - when you want a less ugly output of canonicalize_file_name
          without modifying your client code (and are happy with '/')
        - when you are using canonicalize_file_name to determine
          the...canonical file name...and find that it breaks on
          windows.

and these are the reasons for me to write this patch.

>     In some programs with less techy users, converting '/' to '\\' may also be
>     useful to avoid confusing the user.

These hypothetical programs would certainly be postprocessing
canonicalize_file_name right now.  Especially as it not only returns
file names with "strange" slashes, it even sometimes mixes them.  How
scary!

How would these hypothetical programs that have this exceptional
requirement be impacted negitavely by this patch?  Postprocessing is
always needed.  And if such programse were not hypothetical, then why
didn't they contribute this code to gnulib?

This code is helpful for me, and that I believe that that for most code
(no not all, say 80% of), in most situations (no not all, say 80% of),
having consistency (and probably all slashes) is a much better choice,
and returning an arbitrary mix of slashes is almost always bad for
everyone.

How is this current behaviour:

    system.ini:                   =>  "C:\\windows/system.ini"
    ..////windows/system.ini:     =>  "C:\\windows/system.ini"
    ..\\\\windows\\system.ini:    =>  "C:\\windows/system.ini"
    C:/windows/system.ini:        =>  "C:/windows/system.ini"

i.e., canonicalizing duplicated slashes, but sometimes messing up their
their direction better for the "less techcy users", or the "command line
flag" argument, than returning all slashes?

Greetings,
Janneke

>From e794533715b2979eeeccc753740862d6736897d3 Mon Sep 17 00:00:00 2001
From: "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org>
Date: Fri, 10 Dec 2021 12:08:57 +0100
Subject: [PATCH v2] canonicalize-lgpl: Return file name using '/' consistently
 on MinGW.
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=UTF-8

Before this patch, canonicalize_file_name would return a file name
with seemingly arbitrarily mangled '/' and '\\' under MinGW, depending
on its input, e.g.:

    system.ini:                   =>  "C:\\windows/system.ini"
    ..////windows/system.ini:     =>  "C:\\windows/system.ini"
    ..\\\\windows\\system.ini:    =>  "C:\\windows/system.ini"
    C:/windows/system.ini:        =>  "C:/windows/system.ini"

* lib/canonicalize-lgpl.c (realpath_stk)[__MINGW32__]: Replace replace
any '\\' with '/'.
---
 lib/canonicalize-lgpl.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index 92e9639720..27f55240ef 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -41,6 +41,10 @@
 #include <intprops.h>
 #include <scratch_buffer.h>
 
+#if __MINGW32__
+#include <ctype.h>
+#endif
+
 #ifdef _LIBC
 # include <shlib-compat.h>
 # define GCC_LINT 1
@@ -251,6 +255,15 @@ realpath_stk (const char *name, char *resolved,
             goto error_nomem;
           rname = rname_buf->data;
         }
+#if __MINGW32__
+      char *p = rname;
+      while (*p)
+        {
+          if (ISSLASH (*p))
+            *p = '/';
+          p++;
+        }
+#endif
       dest = __rawmemchr (rname, '\0');
       start = name;
       prefix_len = FILE_SYSTEM_PREFIX_LEN (rname);

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com