Re: [PATCH 2/2] Windows libcpp: Make path-exists semantics more Posix-like
On Fri, Apr 25, 2014 at 9:33 PM, Kai Tietz wrote: > 2014-04-25 21:24 GMT+02:00 Pedro Alves : >> On 04/25/2014 08:05 PM, Kai Tietz wrote: >>> 2014-04-25 18:53 GMT+02:00 Pedro Alves : On 04/19/2014 09:41 PM, Kai Tietz wrote: > Isn't this function something better placed in libiberty? Also this name > looks a bit confusing. Wouldn't be a an function calling for _WIN32 case > also stat, and just overrides the st_mode member, if it is a link better. > So I would put this function to the file_... API of libiberty. I'd even suspect that e.g., GNU Make / Makefiles would be likewise affected by this. A solution for this in gcc, or in a few selected programs only, looks brittle to me. Perhaps it should be mingw itself that provides a _non-default_ replacement as option (similarly to __mingw_printf). >>> >>> Of course we could change default-behavior of stat-function within >>> mingw. >> >> Huh? I said exactly the opposite. To expose it as a __non-default__ >> replacement. I pointed at __mingw_printf, so to suggest programs >> would call it like __mingw_stat or something, or by defining >> __USE_MINGW_POSIX_STAT or something, just like one can define >> __USE_MINGW_ANSI_STDIO before including stdio.h. I'll understand >> if you wouldn't want to support that as an option, but I did _not_ >> suggest making it the default. > > As none-default replacement it makes even less sense in mingw IMO. > the __mingw_printf (and related functions) are there for providing C99 > functionality. What standard shall __mingw_stat satisfy? > >>> I think that libiberty is exactly present to unify functionality (and >>> API) for different operation systems. Exactly for this libiberty was >>> made, isn't it? >> >> libiberty is actually a kitchen sink, and specific to gcc and src. > Well it is shared with binutils. And in the past gdb used it too (I > think it still does in some way, as not everything is in glib. I > might be wrong about this). > >> It does more than host abstraction. Gnulib fills that role much better >> nowdays. I'd be nice if gcc used that instead for the host abstraction >> parts (gdb does), but nobody's working on that afaik... > > That's true for gdb. I don't see that binutils or gcc use glib. So I > can only talk in this thread about what gcc/binutils uses right now. > Actually I am not really interested in what kind of compatibility > library is used. > Nevertheless to hi-jack this thread to start a discussion about > preferring glib over other (already existing) library in gcc/binutils > isn't ok. Please start for such a discussion a separate RFC-thread. > >>> I agree that there are other venture, which might be affected by same >>> problem. So those venture could either use libiberty to solve this >>> problem too, or need to reimplement it as they do now. >> >> And then we'll have reinvented Cygwin all over the map. ;-) > > Huh? mingw != cygwin. and mingw's internal libraries aren't a > kitchen-sink too. > Can't glibc be changed to not rely on this? /me hides. Yes mingw != cygwin, but we'd still like to be able to cross compile glibc on Windows targeting GNU/Linux and this patch is necessary for that to work. I see benefits in normalizing the behavior over three build machine OSes (GNU/Linux, OS X and WIndows) and for all projects being built (rather than just changing glibc). If I hear any positive noises, I'll send a re-based version of the patch. -- Ray >> >> -- >> Pedro Alves > > --- > Kai Tietz
[PATCH] clone_function_name_1: Retain any stdcall suffix
I'm not familiar with setting up GCC testcases yet so I expect to have to do that at least. To aid discussion, the commit message contains a testcase. -- Best regards, Ray. 0022-clone_function_name_1-Retain-any-stdcall-suffix.patch Description: Binary data
[PATCH] Windows libibery: Don't quote args unnecessarily
We only quote arguments that contain spaces, \t or "
characters to prevent wasting 2 characters per
argument of the CreateProcess() 32,768 limit.
---
libiberty/pex-win32.c | 46 +-
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index eae72c5..8b9d4f0 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -340,17 +340,25 @@ argv_to_cmdline (char *const *argv)
char *p;
size_t cmdline_len;
int i, j, k;
+ int needs_quotes;
cmdline_len = 0;
for (i = 0; argv[i]; i++)
{
- /* We quote every last argument. This simplifies the problem;
-we need only escape embedded double-quotes and immediately
+ /* We only quote arguments that contain spaces, \t or " characters to
+prevent wasting 2 chars per argument of the CreateProcess 32k char
+limit. We need only escape embedded double-quotes and immediately
preceeding backslash characters. A sequence of backslach characters
that is not follwed by a double quote character will not be
escaped. */
+ needs_quotes = 0;
for (j = 0; argv[i][j]; j++)
{
+ if (argv[i][j] == ' ' || argv[i][j] == '\t' || argv[i][j] == '"')
+ {
+ needs_quotes = 1;
+ }
+
if (argv[i][j] == '"')
{
/* Escape preceeding backslashes. */
@@ -362,16 +370,33 @@ argv_to_cmdline (char *const *argv)
}
/* Trailing backslashes also need to be escaped because they will be
followed by the terminating quote. */
- for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
- cmdline_len++;
+ if (needs_quotes)
+{
+ for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
+cmdline_len++;
+}
cmdline_len += j;
- cmdline_len += 3; /* for leading and trailing quotes and space */
+ /* for leading and trailing quotes and space */
+ cmdline_len += needs_quotes * 2 + 1;
}
cmdline = XNEWVEC (char, cmdline_len);
p = cmdline;
for (i = 0; argv[i]; i++)
{
- *p++ = '"';
+ needs_quotes = 0;
+ for (j = 0; argv[i][j]; j++)
+{
+ if (argv[i][j] == ' ' || argv[i][j] == '\t' || argv[i][j] == '"')
+{
+ needs_quotes = 1;
+ break;
+}
+}
+
+ if (needs_quotes)
+{
+ *p++ = '"';
+}
for (j = 0; argv[i][j]; j++)
{
if (argv[i][j] == '"')
@@ -382,9 +407,12 @@ argv_to_cmdline (char *const *argv)
}
*p++ = argv[i][j];
}
- for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
- *p++ = '\\';
- *p++ = '"';
+ if (needs_quotes)
+{
+ for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
+*p++ = '\\';
+ *p++ = '"';
+}
*p++ = ' ';
}
p[-1] = '\0';
--
1.9.2
[PATCH] Windows libiberty: Don't quote args unnecessarily (v2)
We only quote arguments that contain spaces, \t or " characters to prevent wasting 2 characters per argument of the CreateProcess() 32,768 limit. libiberty/ * pex-win32.c (argv_to_cmdline): Don't quote args unnecessarily Ray Donnelly (1): Windows libibery: Don't quote args unnecessarily libiberty/pex-win32.c | 46 +- 1 file changed, 37 insertions(+), 9 deletions(-) -- 1.9.2
[PATCH 0/2] Windows: Two improvements.
Patch 1 only quotes arguements where quotes are needed as otherwise the 32k limit can be hit sooner than otherwise. This patch should also be applied to binutils. Patch 2 makes path-exists semantics behave more like Posix in the face of ../ so that (for example) glibc can be cross compiled on Windows. Ray Donnelly (2): Windows libibery: Don't quote args unnecessarily Windows libcpp: Make path-exists semantics more Posix-like libcpp/ChangeLog | 5 +++ libcpp/files.c| 86 +++ libiberty/ChangeLog | 5 +++ libiberty/pex-win32.c | 48 ++-- 4 files changed, 135 insertions(+), 9 deletions(-) -- 1.9.2
[PATCH 1/2] Windows libibery: Don't quote args unnecessarily
We only quote arguments that contain spaces, \n \t \v
or " characters to prevent wasting 2 characters per
argument of the CreateProcess() 32,768 limit.
libiberty/
* pex-win32.c (argv_to_cmdline): Don't quote
args unnecessarily
---
libiberty/ChangeLog | 5 +
libiberty/pex-win32.c | 48 +++-
2 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index d9a208b..f6a4f8f 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,8 @@
+2014-04-14 Ray Donnelly
+
+ * pex-win32.c (argv_to_cmdline): Don't quote
+ args unnecessarily.
+
2014-04-17 Jakub Jelinek
PR sanitizer/56781
diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index eae72c5..775b53c 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -340,17 +340,26 @@ argv_to_cmdline (char *const *argv)
char *p;
size_t cmdline_len;
int i, j, k;
+ int needs_quotes;
cmdline_len = 0;
for (i = 0; argv[i]; i++)
{
- /* We quote every last argument. This simplifies the problem;
-we need only escape embedded double-quotes and immediately
+ /* We only quote arguments that contain spaces, \n \t \v or " characters
+to prevent wasting 2 chars per argument of the CreateProcess 32k char
+limit We need only escape embedded double-quotes and immediately
preceeding backslash characters. A sequence of backslach characters
that is not follwed by a double quote character will not be
escaped. */
+ needs_quotes = 0;
for (j = 0; argv[i][j]; j++)
{
+ if (argv[i][j] == ' ' || argv[i][j] == '\n' ||
+ argv[i][j] == '\t' || argv[i][j] == '"' )
+ {
+ needs_quotes = 1;
+ }
+
if (argv[i][j] == '"')
{
/* Escape preceeding backslashes. */
@@ -362,16 +371,34 @@ argv_to_cmdline (char *const *argv)
}
/* Trailing backslashes also need to be escaped because they will be
followed by the terminating quote. */
- for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
- cmdline_len++;
+ if (needs_quotes)
+{
+ for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
+cmdline_len++;
+}
cmdline_len += j;
- cmdline_len += 3; /* for leading and trailing quotes and space */
+ /* for leading and trailing quotes and space */
+ cmdline_len += needs_quotes * 2 + 1;
}
cmdline = XNEWVEC (char, cmdline_len);
p = cmdline;
for (i = 0; argv[i]; i++)
{
- *p++ = '"';
+ needs_quotes = 0;
+ for (j = 0; argv[i][j]; j++)
+{
+ if (argv[i][j] == ' ' || argv[i][j] == '\n' ||
+ argv[i][j] == '\t' || argv[i][j] == '"' )
+{
+ needs_quotes = 1;
+ break;
+}
+}
+
+ if (needs_quotes)
+{
+ *p++ = '"';
+}
for (j = 0; argv[i][j]; j++)
{
if (argv[i][j] == '"')
@@ -382,9 +409,12 @@ argv_to_cmdline (char *const *argv)
}
*p++ = argv[i][j];
}
- for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
- *p++ = '\\';
- *p++ = '"';
+ if (needs_quotes)
+{
+ for (k = j - 1; k >= 0 && argv[i][k] == '\\'; k--)
+*p++ = '\\';
+ *p++ = '"';
+}
*p++ = ' ';
}
p[-1] = '\0';
--
1.9.2
[PATCH 2/2] Windows libcpp: Make path-exists semantics more Posix-like
Windows does a short-circuit lookup of paths containing
../ which means that:
exists/doesnotexist/../file
is considered to exist, while on Posix it is considered
not to. The Posix semantics are relied upon when building
glibc so any paths containing "../" are checked component
wise.
libcpp/
* files.c (open_file): Implement Posix existence
semantics for paths containing '../'
---
libcpp/ChangeLog | 5
libcpp/files.c | 86
2 files changed, 91 insertions(+)
diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog
index 3a63434..ae1b62a 100644
--- a/libcpp/ChangeLog
+++ b/libcpp/ChangeLog
@@ -1,3 +1,8 @@
+2014-04-14 Ray Donnelly
+
+ * files.c (open_file): Implement Posix existence
+ semantics for paths containing '../'
+
2014-02-24 Walter Lee
* configure.ac: Change "tilepro" triplet to "tilepro*".
diff --git a/libcpp/files.c b/libcpp/files.c
index 7e88778..a9326bf 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -30,6 +30,13 @@ along with this program; see the file COPYING3. If not see
#include "md5.h"
#include
+/* Needed for stat_st_mode_symlink below */
+#if defined(_WIN32)
+# include
+# define S_IFLNK 0xF000
+# define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
+#endif
+
/* Variable length record files on VMS will have a stat size that includes
record control characters that won't be included in the read size. */
#ifdef VMS
@@ -198,6 +205,49 @@ static int pchf_save_compare (const void *e1, const void
*e2);
static int pchf_compare (const void *d_p, const void *e_p);
static bool check_file_against_entries (cpp_reader *, _cpp_file *, bool);
+#if defined(_WIN32)
+
+static int stat_st_mode_symlink (char const* path, struct stat* buf)
+{
+ WIN32_FILE_ATTRIBUTE_DATA attr;
+ memset(buf, 0, sizeof(*buf));
+ int err = GetFileAttributesExA (path, GetFileExInfoStandard, &attr) ? 0 : 1;
+ if (!err)
+{
+ WIN32_FIND_DATAA finddata;
+ HANDLE h = FindFirstFileA (path, &finddata);
+ if (h != INVALID_HANDLE_VALUE)
+{
+ FindClose (h);
+ if ((finddata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) &&
+ (finddata.dwReserved0 == IO_REPARSE_TAG_SYMLINK))
+ buf->st_mode = S_IFLNK;
+ else if (finddata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+ buf->st_mode = S_IFDIR;
+ else if (finddata.dwFileAttributes & FILE_ATTRIBUTE_ARCHIVE)
+ buf->st_mode = S_IFDIR;
+ else
+ buf->st_mode = S_IFREG;
+ buf->st_mode |= S_IREAD;
+ if (!(finddata.dwFileAttributes & FILE_ATTRIBUTE_READONLY))
+ buf->st_mode |= S_IWRITE;
+}
+ else
+{
+ buf->st_mode = S_IFDIR;
+}
+ return 0;
+}
+ return -1;
+}
+
+#else
+
+#define stat_st_mode_symlink (_name, _buf) stat ((_name), (_buf))
+
+#endif
+
+
/* Given a filename in FILE->PATH, with the empty string interpreted
as , open it.
@@ -227,6 +277,42 @@ open_file (_cpp_file *file)
}
else
file->fd = open (file->path, O_RDONLY | O_NOCTTY | O_BINARY, 0666);
+#if defined(_WIN32) || defined(__CYGWIN__)
+ /* Windows and Posix differ in the face of paths of the form:
+ nonexistantdir/.. in that Posix will return ENOENT whereas
+ Windows won't care that we stepped into a non-existant dir
+ Only do these slow checks if "../" appears in file->path.
+ Cygwin also suffers from the same problem (but doesn't need
+ a new stat function):
+ http://cygwin.com/ml/cygwin/2013-05/msg00222.html
+ */
+ if (file->fd > 0)
+{
+ char filepath[MAX_PATH];
+ strncpy (filepath, file->path, MAX_PATH);
+ filepath[MAX_PATH-1] = (char) 0;
+ char *dirsep = &filepath[0];
+ while ( (dirsep = strchr (dirsep, '\\')) != NULL)
+*dirsep = '/';
+ if (strstr(filepath, "../"))
+ {
+ dirsep = &filepath[0];
+ /* Check each directory in the chain. */
+ while ( (dirsep = strchr (dirsep, '/')) != NULL)
+ {
+ *dirsep = (char) 0;
+ if (stat_st_mode_symlink (filepath, &file->st) == -1)
+ {
+ *dirsep = '/';
+ close (file->fd);
+ file->fd = -1;
+ return false;
+ }
+ *dirsep = '/';
+ }
+ }
+}
+#endif
if (file->fd != -1)
{
--
1.9.2
