* [PATCH 0/2] Improve automake object file location for utils/mingw/
@ 2021-05-02 15:25 Jon Turney
2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jon Turney @ 2021-05-02 15:25 UTC (permalink / raw)
To: cygwin-patches; +Cc: Jon Turney
This is the simple part, leaving potentially pulling apart path.cc for later...
Jon Turney (2):
Unpick cygpath TESTSUITE
Move source files used in utils/mingw/ into that subdirectory
winsup/utils/mingw/Makefile.am | 23 +++++++------
winsup/utils/{ => mingw}/bloda.cc | 0
winsup/utils/{ => mingw}/cygcheck.cc | 0
.../{ => mingw}/cygwin-console-helper.cc | 0
winsup/utils/{ => mingw}/dump_setup.cc | 0
winsup/utils/{ => mingw}/ldh.cc | 0
winsup/utils/mingw/path.cc | 1 +
winsup/utils/{ => mingw}/strace.cc | 10 +++---
winsup/utils/{ => mingw}/testsuite.cc | 31 +++++++++--------
winsup/utils/{ => mingw}/testsuite.h | 34 ++++++-------------
winsup/utils/path.cc | 31 +++++++----------
winsup/utils/path.h | 10 ++++--
12 files changed, 67 insertions(+), 73 deletions(-)
rename winsup/utils/{ => mingw}/bloda.cc (100%)
rename winsup/utils/{ => mingw}/cygcheck.cc (100%)
rename winsup/utils/{ => mingw}/cygwin-console-helper.cc (100%)
rename winsup/utils/{ => mingw}/dump_setup.cc (100%)
rename winsup/utils/{ => mingw}/ldh.cc (100%)
create mode 100644 winsup/utils/mingw/path.cc
rename winsup/utils/{ => mingw}/strace.cc (99%)
rename winsup/utils/{ => mingw}/testsuite.cc (85%)
rename winsup/utils/{ => mingw}/testsuite.h (94%)
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] Unpick cygpath TESTSUITE 2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney @ 2021-05-02 15:25 ` Jon Turney 2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney 2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney 2 siblings, 0 replies; 10+ messages in thread From: Jon Turney @ 2021-05-02 15:25 UTC (permalink / raw) To: cygwin-patches; +Cc: Jon Turney Rather than having testsuite.h do various things, depending on defines, just have it do one thing, and then explicitly redirect to test stubs in path.cc when building test. --- winsup/utils/path.cc | 31 +++++++++++++------------------ winsup/utils/path.h | 10 ++++++++-- winsup/utils/testsuite.cc | 31 ++++++++++++++++--------------- winsup/utils/testsuite.h | 34 +++++++++++----------------------- 4 files changed, 48 insertions(+), 58 deletions(-) diff --git a/winsup/utils/path.cc b/winsup/utils/path.cc index 29344be02..0d41a45d8 100644 --- a/winsup/utils/path.cc +++ b/winsup/utils/path.cc @@ -25,7 +25,6 @@ details. */ #include "../cygwin/include/sys/mount.h" #define _NOMNTENT_MACROS #include "../cygwin/include/mntent.h" -#include "testsuite.h" #ifdef FSTAB_ONLY #include <sys/cygwin.h> #endif @@ -255,14 +254,8 @@ readlink (HANDLE fh, char *path, size_t maxlen) } #endif /* !FSTAB_ONLY */ -#ifndef TESTSUITE mnt_t mount_table[255]; int max_mount_entry; -#else -# define TESTSUITE_MOUNT_TABLE -# include "testsuite.h" -# undef TESTSUITE_MOUNT_TABLE -#endif inline void unconvert_slashes (char* name) @@ -271,9 +264,6 @@ unconvert_slashes (char* name) *name++ = '\\'; } -/* These functions aren't called when defined(TESTSUITE) which results - in a compiler warning. */ -#ifndef TESTSUITE inline char * skip_ws (char *in) { @@ -555,11 +545,8 @@ from_fstab (bool user, PWCHAR path, PWCHAR path_end) CloseHandle (h); } #endif /* !FSTAB_ONLY */ -#endif /* !TESTSUITE */ #ifndef FSTAB_ONLY - -#ifndef TESTSUITE static int mnt_sort (const void *a, const void *b) { @@ -653,7 +640,11 @@ read_mounts () from_fstab (true, path, path_end); qsort (mount_table, max_mount_entry, sizeof (mnt_t), mnt_sort); } -#endif /* !defined(TESTSUITE) */ + +#ifdef TESTSUITE +#define read_mounts testsuite_read_mounts +#endif + /* Return non-zero if PATH1 is a prefix of PATH2. Both are assumed to be of the same path style and / vs \ usage. @@ -757,6 +748,11 @@ concat (const char *s, ...) return vconcat (s, v); } +#ifdef TESTSUITE +#undef GetCurrentDirectory +#define GetCurrentDirectory testsuite_getcwd +#endif + /* This is a helper function for when vcygpath is passed what appears to be a relative POSIX path. We take a Win32 CWD (either as specified in 'cwd' or as retrieved with GetCurrentDirectory() if 'cwd' is NULL) @@ -822,10 +818,9 @@ vcygpath (const char *cwd, const char *s, va_list v) size_t max_len = 0; mnt_t *m, *match = NULL; -#ifndef TESTSUITE if (!max_mount_entry) read_mounts (); -#endif + char *path; if (s[0] == '.' && isslash (s[1])) s += 2; @@ -912,10 +907,10 @@ extern "C" FILE * setmntent (const char *, const char *) { m = mount_table; -#ifndef TESTSUITE + if (!max_mount_entry) read_mounts (); -#endif + return NULL; } diff --git a/winsup/utils/path.h b/winsup/utils/path.h index a1840a003..c64f6ebfb 100644 --- a/winsup/utils/path.h +++ b/winsup/utils/path.h @@ -6,6 +6,9 @@ This software is a copyrighted work licensed under the terms of the Cygwin license. Please consult the file "CYGWIN_LICENSE" for details. */ +#ifndef _PATH_H_ +#define _PATH_H_ + struct mnt_t { char *native; @@ -22,11 +25,14 @@ int get_word (HANDLE, int); int get_dword (HANDLE, int); bool from_fstab_line (mnt_t *m, char *line, bool user); -#ifndef TESTSUITE extern mnt_t mount_table[255]; extern int max_mount_entry; -#endif #ifndef SYMLINK_MAX #define SYMLINK_MAX 4095 /* PATH_MAX - 1 */ #endif + +DWORD testsuite_getcwd (DWORD nBufferLength, LPSTR lpBuffer); +void testsuite_read_mounts (void); + +#endif diff --git a/winsup/utils/testsuite.cc b/winsup/utils/testsuite.cc index 23ed8e0d8..ef9f14fa7 100644 --- a/winsup/utils/testsuite.cc +++ b/winsup/utils/testsuite.cc @@ -15,22 +15,9 @@ details. */ #include <unistd.h> #define WIN32_LEAN_AND_MEAN #include <windows.h> -#ifndef TESTSUITE -#define TESTSUITE -#endif +#include "path.h" #include "testsuite.h" -typedef struct - { - const char *cwd; /* in win32 form, as if by GetCurrentDirectory */ - const char *posix; /* input */ - const char *win32; /* expected output */ - } test_t; - -#define TESTSUITE_TESTS -#include "testsuite.h" -#undef TESTSUITE_TESTS - static int curtest; /* A replacement for the w32api GetCurrentDirectory() that returns @@ -55,7 +42,21 @@ testsuite_getcwd (DWORD nBufferLength, LPSTR lpBuffer) return len; } -extern char *cygpath (const char *s, ...); +/* + A replacement for read_mounts that installs the test mount table + */ +void +testsuite_read_mounts (void) +{ + int i; + for (i = 0; test_mount_table[i].posix; i++) + { + mount_table[i] = test_mount_table[i]; + } + max_mount_entry = i; +} + +WCHAR cygwin_dll_path[32768]; int main (int argc, char **argv) diff --git a/winsup/utils/testsuite.h b/winsup/utils/testsuite.h index 0dd631539..c3d9ad60d 100644 --- a/winsup/utils/testsuite.h +++ b/winsup/utils/testsuite.h @@ -6,6 +6,10 @@ This software is a copyrighted work licensed under the terms of the Cygwin license. Please consult the file "CYGWIN_LICENSE" for details. */ +#include "path.h" +#include "../cygwin/include/cygwin/bits.h" +#include "../cygwin/include/sys/mount.h" + /* This file implements a test harness for the MinGW implementation of POSIX path translation in utils/path.cc. This code is used by strace and cygcheck which cannot depend on the Cygwin DLL. The tests below @@ -13,9 +17,6 @@ details. */ absolute paths from POSIX form to Win32 form based on the contents of a mount table. */ -/* Including this file should be a no-op if TESTSUITE is not defined. */ -#ifdef TESTSUITE - /* These definitions are common to both the testsuite mount table as well as the testsuite definitions themselves, so define them here so that they are only defined in one location. */ @@ -26,9 +27,7 @@ details. */ This is used in place of actually reading the host mount table from the registry for the duration of the testsuite. This table should match the battery of tests below. */ - -#if defined(TESTSUITE_MOUNT_TABLE) -static mnt_t mount_table[] = { +static mnt_t test_mount_table[] = { /* native posix flags */ { (char*)TESTSUITE_ROOT, (char*)"/", MOUNT_SYSTEM}, { (char*)"O:\\other", (char*)"/otherdir", MOUNT_SYSTEM}, @@ -39,12 +38,16 @@ static mnt_t mount_table[] = { { NULL, (char*)NULL, 0} }; +typedef struct + { + const char *cwd; /* in win32 form, as if by GetCurrentDirectory */ + const char *posix; /* input */ + const char *win32; /* expected output */ + } test_t; /* Define the main set of tests. This is defined here instead of in testsuite.cc so that all test harness data is in one place and not spread over several files. */ - -#elif defined(TESTSUITE_TESTS) #define NO_CWD "N/A" static test_t testsuite_tests[] = { { NO_CWD, "/file.ext", TESTSUITE_ROOT"\\file.ext" }, @@ -112,18 +115,3 @@ static test_t testsuite_tests[] = { { NO_CWD, "//server/share/foo/bar", "\\\\server\\share\\foo\\bar" }, { NO_CWD, NULL, NULL } }; - -#else - -/* Redirect calls to GetCurrentDirectory() to the testsuite instead. */ -#ifdef GetCurrentDirectory -#undef GetCurrentDirectory -#endif -#define GetCurrentDirectory testsuite_getcwd - -DWORD testsuite_getcwd (DWORD, LPSTR); - -#endif - -#endif /* TESTSUITE */ - -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory 2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney 2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney @ 2021-05-02 15:25 ` Jon Turney 2021-05-03 10:48 ` Corinna Vinschen 2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney 2 siblings, 1 reply; 10+ messages in thread From: Jon Turney @ 2021-05-02 15:25 UTC (permalink / raw) To: cygwin-patches; +Cc: Jon Turney Move all the source files used in utils/mingw/ into that subdirectory, so the built objects are in the expected place. (path.cc requires some more unpicking, and even then there is genuinely some shared code, so use a trivial file which includes the real path.cc so the object file is generated where expected) --- winsup/utils/mingw/Makefile.am | 23 +++++++++++-------- winsup/utils/{ => mingw}/bloda.cc | 0 winsup/utils/{ => mingw}/cygcheck.cc | 0 .../{ => mingw}/cygwin-console-helper.cc | 0 winsup/utils/{ => mingw}/dump_setup.cc | 0 winsup/utils/{ => mingw}/ldh.cc | 0 winsup/utils/mingw/path.cc | 1 + winsup/utils/{ => mingw}/strace.cc | 10 ++++---- winsup/utils/{ => mingw}/testsuite.cc | 0 winsup/utils/{ => mingw}/testsuite.h | 0 10 files changed, 19 insertions(+), 15 deletions(-) rename winsup/utils/{ => mingw}/bloda.cc (100%) rename winsup/utils/{ => mingw}/cygcheck.cc (100%) rename winsup/utils/{ => mingw}/cygwin-console-helper.cc (100%) rename winsup/utils/{ => mingw}/dump_setup.cc (100%) rename winsup/utils/{ => mingw}/ldh.cc (100%) create mode 100644 winsup/utils/mingw/path.cc rename winsup/utils/{ => mingw}/strace.cc (99%) rename winsup/utils/{ => mingw}/testsuite.cc (100%) rename winsup/utils/{ => mingw}/testsuite.h (100%) diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am index a14725902..73abc4264 100644 --- a/winsup/utils/mingw/Makefile.am +++ b/winsup/utils/mingw/Makefile.am @@ -25,26 +25,29 @@ bin_PROGRAMS = \ strace cygcheck_SOURCES = \ - ../bloda.cc \ - ../cygcheck.cc \ - ../dump_setup.cc \ - ../path.cc + bloda.cc \ + cygcheck.cc \ + dump_setup.cc \ + path.cc +cygcheck_CPPFLAGS=-I$(srcdir)/.. cygcheck_LDADD = -lz -lwininet -lpsapi -lntdll -cygwin_console_helper_SOURCES = ../cygwin-console-helper.cc +cygwin_console_helper_SOURCES = cygwin-console-helper.cc -ldh_SOURCES = ../ldh.cc +ldh_SOURCES = ldh.cc strace_SOURCES = \ - ../path.cc \ - ../strace.cc + path.cc \ + strace.cc +strace_CPPFLAGS=-I$(srcdir)/.. strace_LDADD = -lntdll noinst_PROGRAMS = path-testsuite path_testsuite_SOURCES = \ - ../path.cc \ - ../testsuite.cc + path.cc \ + testsuite.cc +path_testsuite_CPPFLAGS=-I$(srcdir)/.. path_testsuite_CXXFLAGS = -DTESTSUITE TESTS = path-testsuite diff --git a/winsup/utils/bloda.cc b/winsup/utils/mingw/bloda.cc similarity index 100% rename from winsup/utils/bloda.cc rename to winsup/utils/mingw/bloda.cc diff --git a/winsup/utils/cygcheck.cc b/winsup/utils/mingw/cygcheck.cc similarity index 100% rename from winsup/utils/cygcheck.cc rename to winsup/utils/mingw/cygcheck.cc diff --git a/winsup/utils/cygwin-console-helper.cc b/winsup/utils/mingw/cygwin-console-helper.cc similarity index 100% rename from winsup/utils/cygwin-console-helper.cc rename to winsup/utils/mingw/cygwin-console-helper.cc diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/mingw/dump_setup.cc similarity index 100% rename from winsup/utils/dump_setup.cc rename to winsup/utils/mingw/dump_setup.cc diff --git a/winsup/utils/ldh.cc b/winsup/utils/mingw/ldh.cc similarity index 100% rename from winsup/utils/ldh.cc rename to winsup/utils/mingw/ldh.cc diff --git a/winsup/utils/mingw/path.cc b/winsup/utils/mingw/path.cc new file mode 100644 index 000000000..dd275b61d --- /dev/null +++ b/winsup/utils/mingw/path.cc @@ -0,0 +1 @@ +#include "../path.cc" diff --git a/winsup/utils/strace.cc b/winsup/utils/mingw/strace.cc similarity index 99% rename from winsup/utils/strace.cc rename to winsup/utils/mingw/strace.cc index b96ad40c1..a7797600c 100644 --- a/winsup/utils/strace.cc +++ b/winsup/utils/mingw/strace.cc @@ -21,11 +21,11 @@ details. */ #include <time.h> #include <signal.h> #include <errno.h> -#include "../cygwin/include/sys/strace.h" -#include "../cygwin/include/sys/cygwin.h" -#include "../cygwin/include/cygwin/version.h" -#include "../cygwin/cygtls_padsize.h" -#include "../cygwin/gcc_seh.h" +#include "../../cygwin/include/sys/strace.h" +#include "../../cygwin/include/sys/cygwin.h" +#include "../../cygwin/include/cygwin/version.h" +#include "../../cygwin/cygtls_padsize.h" +#include "../../cygwin/gcc_seh.h" #include "path.h" #undef cygwin_internal #include "loadlib.h" diff --git a/winsup/utils/testsuite.cc b/winsup/utils/mingw/testsuite.cc similarity index 100% rename from winsup/utils/testsuite.cc rename to winsup/utils/mingw/testsuite.cc diff --git a/winsup/utils/testsuite.h b/winsup/utils/mingw/testsuite.h similarity index 100% rename from winsup/utils/testsuite.h rename to winsup/utils/mingw/testsuite.h -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory 2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney @ 2021-05-03 10:48 ` Corinna Vinschen 2021-05-04 18:34 ` Jon Turney 0 siblings, 1 reply; 10+ messages in thread From: Corinna Vinschen @ 2021-05-03 10:48 UTC (permalink / raw) To: cygwin-patches On May 2 16:25, Jon Turney wrote: > Move all the source files used in utils/mingw/ into that subdirectory, > so the built objects are in the expected place. > > (path.cc requires some more unpicking, and even then there is genuinely > some shared code, so use a trivial file which includes the real path.cc > so the object file is generated where expected) This patchset LGTM, except one thing which isn't your fault: > index b96ad40c1..a7797600c 100644 > --- a/winsup/utils/strace.cc > +++ b/winsup/utils/mingw/strace.cc > @@ -21,11 +21,11 @@ details. */ > #include <time.h> > #include <signal.h> > #include <errno.h> > -#include "../cygwin/include/sys/strace.h" > -#include "../cygwin/include/sys/cygwin.h" > -#include "../cygwin/include/cygwin/version.h" > -#include "../cygwin/cygtls_padsize.h" > -#include "../cygwin/gcc_seh.h" > +#include "../../cygwin/include/sys/strace.h" > +#include "../../cygwin/include/sys/cygwin.h" > +#include "../../cygwin/include/cygwin/version.h" > +#include "../../cygwin/cygtls_padsize.h" > +#include "../../cygwin/gcc_seh.h" What about adding -I../../cygwin -I../../cygwin/include to the build rules and get rid of the relative paths inside the sources? Thanks, Corinna ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory 2021-05-03 10:48 ` Corinna Vinschen @ 2021-05-04 18:34 ` Jon Turney 2021-05-06 8:43 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Jon Turney @ 2021-05-04 18:34 UTC (permalink / raw) To: Cygwin Patches On 03/05/2021 11:48, Corinna Vinschen wrote: > On May 2 16:25, Jon Turney wrote: >> Move all the source files used in utils/mingw/ into that subdirectory, >> so the built objects are in the expected place. >> >> (path.cc requires some more unpicking, and even then there is genuinely >> some shared code, so use a trivial file which includes the real path.cc >> so the object file is generated where expected) > > This patchset LGTM, except one thing which isn't your fault: > >> index b96ad40c1..a7797600c 100644 >> --- a/winsup/utils/strace.cc >> +++ b/winsup/utils/mingw/strace.cc >> @@ -21,11 +21,11 @@ details. */ >> #include <time.h> >> #include <signal.h> >> #include <errno.h> >> -#include "../cygwin/include/sys/strace.h" >> -#include "../cygwin/include/sys/cygwin.h" >> -#include "../cygwin/include/cygwin/version.h" >> -#include "../cygwin/cygtls_padsize.h" >> -#include "../cygwin/gcc_seh.h" >> +#include "../../cygwin/include/sys/strace.h" >> +#include "../../cygwin/include/sys/cygwin.h" >> +#include "../../cygwin/include/cygwin/version.h" >> +#include "../../cygwin/cygtls_padsize.h" >> +#include "../../cygwin/gcc_seh.h" > > What about adding -I../../cygwin -I../../cygwin/include to the build > rules and get rid of the relative paths inside the sources? That seems fraught as it allows cygwin system headers to be picked up in preference to mingw ones? Using '-idirafter' gets you a build, but it would be much more work to check that you've actually built what you wanted to... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory 2021-05-04 18:34 ` Jon Turney @ 2021-05-06 8:43 ` Corinna Vinschen 2021-05-09 15:16 ` Jon Turney 0 siblings, 1 reply; 10+ messages in thread From: Corinna Vinschen @ 2021-05-06 8:43 UTC (permalink / raw) To: cygwin-patches On May 4 19:34, Jon Turney wrote: > On 03/05/2021 11:48, Corinna Vinschen wrote: > > On May 2 16:25, Jon Turney wrote: > > > Move all the source files used in utils/mingw/ into that subdirectory, > > > so the built objects are in the expected place. > > > > > > (path.cc requires some more unpicking, and even then there is genuinely > > > some shared code, so use a trivial file which includes the real path.cc > > > so the object file is generated where expected) > > > > This patchset LGTM, except one thing which isn't your fault: > > > > > index b96ad40c1..a7797600c 100644 > > > --- a/winsup/utils/strace.cc > > > +++ b/winsup/utils/mingw/strace.cc > > > @@ -21,11 +21,11 @@ details. */ > > > #include <time.h> > > > #include <signal.h> > > > #include <errno.h> > > > -#include "../cygwin/include/sys/strace.h" > > > -#include "../cygwin/include/sys/cygwin.h" > > > -#include "../cygwin/include/cygwin/version.h" > > > -#include "../cygwin/cygtls_padsize.h" > > > -#include "../cygwin/gcc_seh.h" > > > +#include "../../cygwin/include/sys/strace.h" > > > +#include "../../cygwin/include/sys/cygwin.h" > > > +#include "../../cygwin/include/cygwin/version.h" > > > +#include "../../cygwin/cygtls_padsize.h" > > > +#include "../../cygwin/gcc_seh.h" > > > > What about adding -I../../cygwin -I../../cygwin/include to the build > > rules and get rid of the relative paths inside the sources? > > That seems fraught as it allows cygwin system headers to be picked up in > preference to mingw ones? > > Using '-idirafter' gets you a build, but it would be much more work to check > that you've actually built what you wanted to... Well, ok. It just looks *so* ugly... What about at least --idirafter ../../cygwin and then #include "include/sys/strace.h" #include "include/sys/cygwin.h" #include "include/cygwin/version.h" #include "cygtls_padsize.h" #include "gcc_seh.h" That would disallow picking up system headers and still be a bit cleaner, no? Corinna ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory 2021-05-06 8:43 ` Corinna Vinschen @ 2021-05-09 15:16 ` Jon Turney 2021-05-10 7:51 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Jon Turney @ 2021-05-09 15:16 UTC (permalink / raw) To: Cygwin Patches On 06/05/2021 09:43, Corinna Vinschen wrote: > On May 4 19:34, Jon Turney wrote: >> On 03/05/2021 11:48, Corinna Vinschen wrote: >>> On May 2 16:25, Jon Turney wrote: >>>> Move all the source files used in utils/mingw/ into that subdirectory, >>>> so the built objects are in the expected place. >>>> >>>> (path.cc requires some more unpicking, and even then there is genuinely >>>> some shared code, so use a trivial file which includes the real path.cc >>>> so the object file is generated where expected) >>> >>> This patchset LGTM, except one thing which isn't your fault: >>> >>>> index b96ad40c1..a7797600c 100644 >>>> --- a/winsup/utils/strace.cc >>>> +++ b/winsup/utils/mingw/strace.cc >>>> @@ -21,11 +21,11 @@ details. */ >>>> #include <time.h> >>>> #include <signal.h> >>>> #include <errno.h> >>>> -#include "../cygwin/include/sys/strace.h" >>>> -#include "../cygwin/include/sys/cygwin.h" >>>> -#include "../cygwin/include/cygwin/version.h" >>>> -#include "../cygwin/cygtls_padsize.h" >>>> -#include "../cygwin/gcc_seh.h" >>>> +#include "../../cygwin/include/sys/strace.h" >>>> +#include "../../cygwin/include/sys/cygwin.h" >>>> +#include "../../cygwin/include/cygwin/version.h" >>>> +#include "../../cygwin/cygtls_padsize.h" >>>> +#include "../../cygwin/gcc_seh.h" >>> >>> What about adding -I../../cygwin -I../../cygwin/include to the build >>> rules and get rid of the relative paths inside the sources? >> >> That seems fraught as it allows cygwin system headers to be picked up in >> preference to mingw ones? >> >> Using '-idirafter' gets you a build, but it would be much more work to check >> that you've actually built what you wanted to... > > Well, ok. It just looks *so* ugly... What about at least > > --idirafter ../../cygwin > > and then > > #include "include/sys/strace.h" > #include "include/sys/cygwin.h" > #include "include/cygwin/version.h" > #include "cygtls_padsize.h" > #include "gcc_seh.h" > > That would disallow picking up system headers and still be a bit > cleaner, no? After thinking about this a bit more, I'm fairly certain that using -idirafter with both paths gets us the same build as before, so I've posted a patch with that change. However, as written it's still a bit dangerous: any includes of system headers by those files included from winsup/cygwin will be getting MinGW system headers. I don't think that e.g. the value of ULONG_MAX is going be used by any of those, but there is a theoretical risk of them not getting what is expected... Perhaps the only safe way to write this is to put the numeric constants which strace uses into a separate header. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory 2021-05-09 15:16 ` Jon Turney @ 2021-05-10 7:51 ` Corinna Vinschen 0 siblings, 0 replies; 10+ messages in thread From: Corinna Vinschen @ 2021-05-10 7:51 UTC (permalink / raw) To: cygwin-patches On May 9 16:16, Jon Turney wrote: > On 06/05/2021 09:43, Corinna Vinschen wrote: > > On May 4 19:34, Jon Turney wrote: > > > On 03/05/2021 11:48, Corinna Vinschen wrote: > > > > What about adding -I../../cygwin -I../../cygwin/include to the build > > > > rules and get rid of the relative paths inside the sources? > > > > > > That seems fraught as it allows cygwin system headers to be picked up in > > > preference to mingw ones? > > > > > > Using '-idirafter' gets you a build, but it would be much more work to check > > > that you've actually built what you wanted to... > > > > Well, ok. It just looks *so* ugly... What about at least > > > > --idirafter ../../cygwin > > > > and then > > > > #include "include/sys/strace.h" > > #include "include/sys/cygwin.h" > > #include "include/cygwin/version.h" > > #include "cygtls_padsize.h" > > #include "gcc_seh.h" > > That would disallow picking up system headers and still be a bit > > cleaner, no? > > After thinking about this a bit more, I'm fairly certain that using > -idirafter with both paths gets us the same build as before, so I've posted > a patch with that change. > > However, as written it's still a bit dangerous: any includes of system > headers by those files included from winsup/cygwin will be getting MinGW > system headers. I don't think that e.g. the value of ULONG_MAX is going be > used by any of those, but there is a theoretical risk of them not getting > what is expected... > > Perhaps the only safe way to write this is to put the numeric constants > which strace uses into a separate header. That sounds like a really good idea. Corinna ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/2] Get rid of relative include paths in strace.cc 2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney 2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney 2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney @ 2021-05-09 15:09 ` Jon Turney 2021-05-10 8:09 ` Corinna Vinschen 2 siblings, 1 reply; 10+ messages in thread From: Jon Turney @ 2021-05-09 15:09 UTC (permalink / raw) To: cygwin-patches; +Cc: Jon Turney --- winsup/utils/mingw/Makefile.am | 2 +- winsup/utils/mingw/strace.cc | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am index 73abc4264..874dce512 100644 --- a/winsup/utils/mingw/Makefile.am +++ b/winsup/utils/mingw/Makefile.am @@ -39,7 +39,7 @@ ldh_SOURCES = ldh.cc strace_SOURCES = \ path.cc \ strace.cc -strace_CPPFLAGS=-I$(srcdir)/.. +strace_CPPFLAGS=-I$(srcdir)/.. -idirafter ${top_srcdir}/cygwin -idirafter ${top_srcdir}/cygwin/include strace_LDADD = -lntdll noinst_PROGRAMS = path-testsuite diff --git a/winsup/utils/mingw/strace.cc b/winsup/utils/mingw/strace.cc index a7797600c..d8a095fb6 100644 --- a/winsup/utils/mingw/strace.cc +++ b/winsup/utils/mingw/strace.cc @@ -21,11 +21,11 @@ details. */ #include <time.h> #include <signal.h> #include <errno.h> -#include "../../cygwin/include/sys/strace.h" -#include "../../cygwin/include/sys/cygwin.h" -#include "../../cygwin/include/cygwin/version.h" -#include "../../cygwin/cygtls_padsize.h" -#include "../../cygwin/gcc_seh.h" +#include "sys/strace.h" +#include "sys/cygwin.h" +#include "cygwin/version.h" +#include "cygtls_padsize.h" +#include "gcc_seh.h" #include "path.h" #undef cygwin_internal #include "loadlib.h" -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/2] Get rid of relative include paths in strace.cc 2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney @ 2021-05-10 8:09 ` Corinna Vinschen 0 siblings, 0 replies; 10+ messages in thread From: Corinna Vinschen @ 2021-05-10 8:09 UTC (permalink / raw) To: cygwin-patches On May 9 16:09, Jon Turney wrote: > --- > winsup/utils/mingw/Makefile.am | 2 +- > winsup/utils/mingw/strace.cc | 10 +++++----- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am > index 73abc4264..874dce512 100644 > --- a/winsup/utils/mingw/Makefile.am > +++ b/winsup/utils/mingw/Makefile.am > @@ -39,7 +39,7 @@ ldh_SOURCES = ldh.cc > strace_SOURCES = \ > path.cc \ > strace.cc > -strace_CPPFLAGS=-I$(srcdir)/.. > +strace_CPPFLAGS=-I$(srcdir)/.. -idirafter ${top_srcdir}/cygwin -idirafter ${top_srcdir}/cygwin/include > strace_LDADD = -lntdll > > noinst_PROGRAMS = path-testsuite > diff --git a/winsup/utils/mingw/strace.cc b/winsup/utils/mingw/strace.cc > index a7797600c..d8a095fb6 100644 > --- a/winsup/utils/mingw/strace.cc > +++ b/winsup/utils/mingw/strace.cc > @@ -21,11 +21,11 @@ details. */ > #include <time.h> > #include <signal.h> > #include <errno.h> > -#include "../../cygwin/include/sys/strace.h" > -#include "../../cygwin/include/sys/cygwin.h" > -#include "../../cygwin/include/cygwin/version.h" > -#include "../../cygwin/cygtls_padsize.h" > -#include "../../cygwin/gcc_seh.h" > +#include "sys/strace.h" > +#include "sys/cygwin.h" > +#include "cygwin/version.h" > +#include "cygtls_padsize.h" > +#include "gcc_seh.h" > #include "path.h" > #undef cygwin_internal > #include "loadlib.h" > -- > 2.31.1 Great, please push. Thanks, Corinna ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-05-10 8:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-02 15:25 [PATCH 0/2] Improve automake object file location for utils/mingw/ Jon Turney 2021-05-02 15:25 ` [PATCH 1/2] Unpick cygpath TESTSUITE Jon Turney 2021-05-02 15:25 ` [PATCH 2/2] Move source files used in utils/mingw/ into that subdirectory Jon Turney 2021-05-03 10:48 ` Corinna Vinschen 2021-05-04 18:34 ` Jon Turney 2021-05-06 8:43 ` Corinna Vinschen 2021-05-09 15:16 ` Jon Turney 2021-05-10 7:51 ` Corinna Vinschen 2021-05-09 15:09 ` [PATCH 3/2] Get rid of relative include paths in strace.cc Jon Turney 2021-05-10 8:09 ` Corinna Vinschen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).