{"thread":{"id":"31233","subject":"[PATCH] help: correct behavior for is_executable on Windows","startedAt":"2012-08-11T07:00:30Z","lastAt":"2012-08-16T16:35:43Z","messageCount":11,"participants":["Heiko Voigt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196855","messageId":"20120811070030.GA83665@book.hvoigt.net","threadId":"31233","inReplyTo":null,"subject":"[PATCH] help: correct behavior for is_executable on Windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2012-08-11T07:00:30Z","receivedAt":"2012-08-11T07:00:30Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"The previous implementation said that the filesystem information on\nWindows is not reliable to determine whether a file is executable.\nTo find gather this information it was peeking into the first two bytes\nof a file to see whether it looks executable.\nApart from the fact that on Windows executables are usually defined as\nsuch by their extension it lead to slow opening of help file in some\nsituations.\n\nWhen you have virus scanner running calling open on an executable file\nis a potentially expensive operation. See the following measurements (in\nseconds) for example.\n\nWith virus scanner running (coldcache):\n\n$ ./a.exe /libexec/git-core/\nbefore open (git-add.exe): 0.000000\nafter open (git-add.exe): 0.412873\nbefore open (git-annotate.exe): 0.000175\nafter open (git-annotate.exe): 0.397925\nbefore open (git-apply.exe): 0.000243\nafter open (git-apply.exe): 0.399996\nbefore open (git-archive.exe): 0.000147\nafter open (git-archive.exe): 0.397783\nbefore open (git-bisect--helper.exe): 0.000160\nafter open (git-bisect--helper.exe): 0.397700\nbefore open (git-blame.exe): 0.000160\nafter open (git-blame.exe): 0.399136\n...\n\nWith virus scanner running (hotcache):\n\n$ ./a.exe /libexec/git-core/\nbefore open (git-add.exe): 0.000000\nafter open (git-add.exe): 0.000325\nbefore open (git-annotate.exe): 0.000229\nafter open (git-annotate.exe): 0.000177\nbefore open (git-apply.exe): 0.000167\nafter open (git-apply.exe): 0.000150\nbefore open (git-archive.exe): 0.000154\nafter open (git-archive.exe): 0.000156\nbefore open (git-bisect--helper.exe): 0.000132\nafter open (git-bisect--helper.exe): 0.000180\nbefore open (git-blame.exe): 0.000718\nafter open (git-blame.exe): 0.000724\n...\n\nThis test did just list the given directory and open() each file in it.\n\nWith this patch I get:\n\n$ time git help git\nLaunching default browser to display HTML ...\n\nreal 0m8.723s\nuser 0m0.000s\nsys 0m0.000s\n\nand without\n\n$ time git help git\nLaunching default browser to display HTML ...\n\nreal 1m37.734s\nuser 0m0.000s\nsys 0m0.031s\n\nboth tests with cold cache and giving the machine some time to settle\ndown after restart.\n\nSigned-off-by: Heiko Voigt \n---\n help.c | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 662349d..b41fa21 100644\n--- a/help.c\n+++ b/help.c\n@@ -103,10 +103,19 @@ static int is_executable(const char *name)\n \t\treturn 0;\n \n #if defined(WIN32) || defined(__CYGWIN__)\n+\t/* On Windows we cannot use the executable bit. The executable\n+\t * state is determined by extension only. We do this first\n+\t * because with virus scanners opening an executeable for\n+\t * reading is potentially expensive.\n+\t */\n+\tif (has_extension(name, \".exe\"))\n+\t\treturn S_IXUSR;\n+\n #if defined(__CYGWIN__)\n if ((st.st_mode & S_IXUSR) == 0)\n #endif\n-{\t/* cannot trust the executable bit, peek into the file instead */\n+{\t/* now that we know it does not have an executable extension,\n+\t peek into the file instead */\n \tchar buf[3] = { 0 };\n \tint n;\n \tint fd = open(name, O_RDONLY);\n@@ -114,8 +123,8 @@ if ((st.st_mode & S_IXUSR) == 0)\n \tif (fd >= 0) {\n \t\tn = read(fd, buf, 2);\n \t\tif (n == 2)\n-\t\t\t/* DOS executables start with \"MZ\" */\n-\t\t\tif (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n+\t\t\t/* look for a she-bang */\n+\t\t\tif (!strcmp(buf, \"#!\"))\n \t\t\t\tst.st_mode |= S_IXUSR;\n \t\tclose(fd);\n \t}\n-- \n1.7.12.rc2.10.gaf2525e\n"},{"id":"196876","messageId":"7vd32whgvl.fsf@alter.siamese.dyndns.org","threadId":"31233","inReplyTo":"20120811070030.GA83665@book.hvoigt.net","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-12T04:30:06Z","receivedAt":"2012-08-12T04:30:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> help.c | 15 ++++++++++++---\n> 1 file changed, 12 insertions(+), 3 deletions(-)\n>\n> diff --git a/help.c b/help.c\n> index 662349d..b41fa21 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -103,10 +103,19 @@ static int is_executable(const char *name)\n> \t\treturn 0;\n> \n> #if defined(WIN32) || defined(__CYGWIN__)\n> +\t/* On Windows we cannot use the executable bit. The executable\n> +\t * state is determined by extension only. We do this first\n> +\t * because with virus scanners opening an executeable for\n> +\t * reading is potentially expensive.\n> +\t */\n> +\tif (has_extension(name, \".exe\"))\n> +\t\treturn S_IXUSR;\n> +\n> #if defined(__CYGWIN__)\n> if ((st.st_mode & S_IXUSR) == 0)\n> #endif\n> -{\t/* cannot trust the executable bit, peek into the file instead */\n> +{\t/* now that we know it does not have an executable extension,\n> +\t peek into the file instead */\n> \tchar buf[3] = { 0 };\n> \tint n;\n> \tint fd = open(name, O_RDONLY);\n> @@ -114,8 +123,8 @@ if ((st.st_mode & S_IXUSR) == 0)\n> \tif (fd >= 0) {\n> \t\tn = read(fd, buf, 2);\n> \t\tif (n == 2)\n> -\t\t\t/* DOS executables start with \"MZ\" */\n> -\t\t\tif (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n> +\t\t\t/* look for a she-bang */\n> +\t\t\tif (!strcmp(buf, \"#!\"))\n> \t\t\t\tst.st_mode |= S_IXUSR;\n> \t\tclose(fd);\n> \t}\n\nWould it make sense to move this to compat/win32/, compat/cygwin.c,\nand compat/posix.c, each exporting is_executable(const char *path),\nso that we do not have to suffer the #ifdef mess?\n"},{"id":"196916","messageId":"20120813170221.GB6418@book.hvoigt.net","threadId":"31233","inReplyTo":"7vd32whgvl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2012-08-13T17:02:23Z","receivedAt":"2012-08-13T17:02:23Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Sat, Aug 11, 2012 at 09:30:06PM -0700, Junio C Hamano wrote:\n> Heiko Voigt writes:\n> \n> > help.c | 15 ++++++++++++---\n> > 1 file changed, 12 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/help.c b/help.c\n> > index 662349d..b41fa21 100644\n> > --- a/help.c\n> > +++ b/help.c\n> > @@ -103,10 +103,19 @@ static int is_executable(const char *name)\n> > \t\treturn 0;\n> > \n> > #if defined(WIN32) || defined(__CYGWIN__)\n> > +\t/* On Windows we cannot use the executable bit. The executable\n> > +\t * state is determined by extension only. We do this first\n> > +\t * because with virus scanners opening an executeable for\n> > +\t * reading is potentially expensive.\n> > +\t */\n> > +\tif (has_extension(name, \".exe\"))\n> > +\t\treturn S_IXUSR;\n> > +\n> > #if defined(__CYGWIN__)\n> > if ((st.st_mode & S_IXUSR) == 0)\n> > #endif\n> > -{\t/* cannot trust the executable bit, peek into the file instead */\n> > +{\t/* now that we know it does not have an executable extension,\n> > +\t peek into the file instead */\n> > \tchar buf[3] = { 0 };\n> > \tint n;\n> > \tint fd = open(name, O_RDONLY);\n> > @@ -114,8 +123,8 @@ if ((st.st_mode & S_IXUSR) == 0)\n> > \tif (fd >= 0) {\n> > \t\tn = read(fd, buf, 2);\n> > \t\tif (n == 2)\n> > -\t\t\t/* DOS executables start with \"MZ\" */\n> > -\t\t\tif (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n> > +\t\t\t/* look for a she-bang */\n> > +\t\t\tif (!strcmp(buf, \"#!\"))\n> > \t\t\t\tst.st_mode |= S_IXUSR;\n> > \t\tclose(fd);\n> > \t}\n> \n> Would it make sense to move this to compat/win32/, compat/cygwin.c,\n> and compat/posix.c, each exporting is_executable(const char *path),\n> so that we do not have to suffer the #ifdef mess?\n\nYes that makes sense. But that means I need to test the code on multiple\nplatforms. To ease the merge in msysgit (the patch is already applied there)\nI would suggest to post a follow up patch which would split up the function\ninto the platform specific parts.\n\nSince the code for cygwin and windows in general is almost the same I would\nextract one function for them where I leave in one ifdef for cygwin.\n\nE.g. like this:\n\n\n\tstatic int is_executable(const char *name)\n\t{\n\t struct stat st;\n\n\t if (stat(name, &st) || /* stat, not lstat */\n\t !S_ISREG(st.st_mode))\n\t return 0;\n\n\t\tfill_platform_stat(name, &st);\n\n\t return st.st_mode & S_IXUSR;\n\t}\n\nwhich I could then define to a no op on posix. That way we avoid code\nduplication in the platform specific functions.\n\nWhat do you think?\n\nCheers Heiko\n"},{"id":"196922","messageId":"7vmx1yel9d.fsf@alter.siamese.dyndns.org","threadId":"31233","inReplyTo":"20120813170221.GB6418@book.hvoigt.net","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-13T17:48:14Z","receivedAt":"2012-08-13T17:48:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> Since the code for cygwin and windows in general is almost the same I would\n> extract one function for them where I leave in one ifdef for cygwin.\n>\n> E.g. like this:\n>\n>\n> \tstatic int is_executable(const char *name)\n> \t{\n> \t struct stat st;\n>\n> \t if (stat(name, &st) || /* stat, not lstat */\n> \t !S_ISREG(st.st_mode))\n> \t return 0;\n>\n> \t\tfill_platform_stat(name, &st);\n>\n> \t return st.st_mode & S_IXUSR;\n> \t}\n>\n> which I could then define to a no op on posix. That way we avoid code\n> duplication in the platform specific functions.\n>\n> What do you think?\n\nDoes having the \"stat()\" help on Windows in any way? Does it ever\nreturn an executable bit by itself?\n\nIf not, Windows compat/ implementation may want to skip issuing a\nuseless stat() and write it as\n\n\tif (has_extension(\".exe\"))\n \treturn 1;\n\tif (contents_begins_with(\"MZ\") || contents_begins_with(\"#!\"))\n \treturn 1;\n\treturn 0;\n\nwithout ever talking about stat() which is POSIXism compat/\nimplementation for Windows does not have to worry about.\n\nAnd that was the reason I suggested making the whole implementation\nof path_is_executable() overridable by compat/ layer.\n\nBut if having \"stat()\" helps on Windows, then your counterproposal\nis good enough for me.\n\nThanks.\n"},{"id":"197042","messageId":"20120815165054.GA43523@book.hvoigt.net","threadId":"31233","inReplyTo":"7vmx1yel9d.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2012-08-15T16:50:55Z","receivedAt":"2012-08-15T16:50:55Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi Junio,\n\nOn Mon, Aug 13, 2012 at 10:48:14AM -0700, Junio C Hamano wrote:\n> Heiko Voigt writes:\n> > What do you think?\n> \n> Does having the \"stat()\" help on Windows in any way? Does it ever\n> return an executable bit by itself?\n\nNo, AFAIK it does not return anything about executability. But I think\nthe stat is still necessary to verify that the file exists and is a\nregular file.\n\nHere is a draft of a patch I would follow up with:\n\nSubject: [PATCH RFC] help: extract platform dependent part of is_executable in\n extra module\n\nTo remove various ifdefs required for the special handling of\nexecutables on windows we create a new module 'executable' in compat\nwhich allows a user to correctly fill the S_IXUSR flag of struct stat on\nWindows.\n\nSince this is valid for all variants of windows (mingw, msvc, cygwin) we\ncreate a new module to avoid code duplication. compat/msvc.h includes\nmingw.h so we do not add an extra include there. By default we define\ncorrect_executeable_stat() to a no op on all other platforms. This is\nguarded by a NO_EXECUTABLE_STAT which when defined by a compat header\nrequires and implementation of this function.\n\nNOTE: I did not test this. I first would like to discuss whether the\noverall structure this approach is taking is ok.\n\nSigned-off-by: Heiko Voigt \n---\n Makefile | 8 +++++---\n compat/cygwin.h | 2 ++\n compat/mingw.h | 2 ++\n compat/win32/executable.c | 33 +++++++++++++++++++++++++++++++++\n compat/win32/executable.h | 9 +++++++++\n git-compat-util.h | 4 ++++\n help.c | 30 ++----------------------------\n 7 files changed, 57 insertions(+), 31 deletions(-)\n create mode 100644 compat/win32/executable.c\n create mode 100644 compat/win32/executable.h\n\ndiff --git a/Makefile b/Makefile\nindex 6b0c961..cea3559 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1073,7 +1073,7 @@ ifeq ($(uname_O),Cygwin)\n \t# Try commenting this out if you suspect MMAP is more efficient\n \tNO_MMAP = YesPlease\n \tX = .exe\n-\tCOMPAT_OBJS += compat/cygwin.o\n+\tCOMPAT_OBJS += compat/cygwin.o compat/win32/executable.o\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n \tSPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield\n endif\n@@ -1257,7 +1257,8 @@ ifeq ($(uname_S),Windows)\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n \tCOMPAT_OBJS = compat/msvc.o compat/winansi.o \\\n \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n-\t\tcompat/win32/poll.o compat/win32/dirent.o\n+\t\tcompat/win32/poll.o compat/win32/dirent.o \\\n+\t\tcompat/win32/executable.o\n \tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib\n@@ -1347,7 +1348,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n \tCOMPAT_OBJS += compat/mingw.o compat/winansi.o \\\n \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n-\t\tcompat/win32/poll.o compat/win32/dirent.o\n+\t\tcompat/win32/poll.o compat/win32/dirent.o \\\n+\t\tcompat/win32/executable.o\n \tEXTLIBS += -lws2_32\n \tPTHREAD_LIBS =\n \tX = .exe\ndiff --git a/compat/cygwin.h b/compat/cygwin.h\nindex a3229f5..e3bbd4d 100644\n--- a/compat/cygwin.h\n+++ b/compat/cygwin.h\n@@ -1,6 +1,8 @@\n #include \n #include \n \n+#include \"win32/executable.h\"\n+\n typedef int (*stat_fn_t)(const char*, struct stat*);\n extern stat_fn_t cygwin_stat_fn;\n extern stat_fn_t cygwin_lstat_fn;\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 61a6521..73c4f3d 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -1,6 +1,8 @@\n #include \n #include \n \n+#include \"win32/executable.h\"\n+\n /*\n * things that are not available in header files\n */\ndiff --git a/compat/win32/executable.c b/compat/win32/executable.c\nnew file mode 100644\nindex 0000000..326ddb1\n--- /dev/null\n+++ b/compat/win32/executable.c\n@@ -0,0 +1,33 @@\n+#include \"executeable.h\"\n+\n+void correct_executable_stat(const char *filename, struct stat *st)\n+{\n+\tchar buf[3] = { 0 };\n+\tint n, fd;\n+\n+\t/* On Windows we cannot use the executable bit. The executable\n+\t * state is determined by extension only. We do this first\n+\t * because with virus scanners opening an executeable for\n+\t * reading is potentially expensive.\n+\t */\n+\tif (has_extension(name, \".exe\"))\n+\t\tst->st_mode |= S_IXUSR;\n+\n+\tif (st.st_mode & S_IXUSR)\n+\t\treturn;\n+\n+\t/* now that we know it does not have an executable extension,\n+\t peek into the file instead */\n+\tfd = open(name, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn;\n+\n+\tn = read(fd, buf, 2);\n+\tif (n == 2) {\n+\t\t/* look for a she-bang */\n+\t\tif (!strcmp(buf, \"#!\"))\n+\t\t\tst.st_mode |= S_IXUSR;\n+\t}\n+\n+\tclose(fd);\n+}\ndiff --git a/compat/win32/executable.h b/compat/win32/executable.h\nnew file mode 100644\nindex 0000000..e4f0c5c\n--- /dev/null\n+++ b/compat/win32/executable.h\n@@ -0,0 +1,9 @@\n+#ifndef COMPAT_WIN32_EXECUTEABLE\n+#define COMPAT_WIN32_EXECUTEABLE\n+\n+#include \"../../git-compat-util.h\"\n+\n+#define NO_EXECUTABLE_STAT\n+extern void correct_executable_stat(const char *filename, struct stat *st);\n+\n+#endif /* COMPAT_WIN32_EXECUTEABLE */\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 35b095e..1b723af 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -607,4 +607,8 @@ int remove_or_warn(unsigned int mode, const char *path);\n /* Get the passwd entry for the UID of the current process. */\n struct passwd *xgetpwuid_self(void);\n \n+#ifndef NO_EXECUTABLE_STAT\n+#define correct_executable_stat(name, stat) ;\n+#endif\n+\n #endif\ndiff --git a/help.c b/help.c\nindex ebc2c42..d9fae3c 100644\n--- a/help.c\n+++ b/help.c\n@@ -106,34 +106,8 @@ static int is_executable(const char *name)\n \t !S_ISREG(st.st_mode))\n \t\treturn 0;\n \n-#if defined(WIN32) || defined(__CYGWIN__)\n-\t/* On Windows we cannot use the executable bit. The executable\n-\t * state is determined by extension only. We do this first\n-\t * because with virus scanners opening an executeable for\n-\t * reading is potentially expensive.\n-\t */\n-\tif (has_extension(name, \".exe\"))\n-\t\treturn S_IXUSR;\n-\n-#if defined(__CYGWIN__)\n-if ((st.st_mode & S_IXUSR) == 0)\n-#endif\n-{\t/* now that we know it does not have an executable extension,\n-\t peek into the file instead */\n-\tchar buf[3] = { 0 };\n-\tint n;\n-\tint fd = open(name, O_RDONLY);\n-\tst.st_mode &= ~S_IXUSR;\n-\tif (fd >= 0) {\n-\t\tn = read(fd, buf, 2);\n-\t\tif (n == 2)\n-\t\t\t/* look for a she-bang */\n-\t\t\tif (!strcmp(buf, \"#!\"))\n-\t\t\t\tst.st_mode |= S_IXUSR;\n-\t\tclose(fd);\n-\t}\n-}\n-#endif\n+\tcorrect_executable_stat(name, &st);\n+\n \treturn st.st_mode & S_IXUSR;\n }\n \n-- \n1.7.12.rc2.11.g5d52328\n"},{"id":"197047","messageId":"7v7gt06nyk.fsf@alter.siamese.dyndns.org","threadId":"31233","inReplyTo":"20120815165054.GA43523@book.hvoigt.net","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T17:53:55Z","receivedAt":"2012-08-15T17:53:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> On Mon, Aug 13, 2012 at 10:48:14AM -0700, Junio C Hamano wrote:\n>> Heiko Voigt writes:\n>> > What do you think?\n>> \n>> Does having the \"stat()\" help on Windows in any way? Does it ever\n>> return an executable bit by itself?\n>\n> No, AFAIK it does not return anything about executability. But I think\n> the stat is still necessary to verify that the file exists and is a\n> regular file.\n\nBut if you are going to read it anyway, you can tell it from open()\nand read() of the first 2 bytes failing, no? That will still be an\nimplementation detail of platform specific \"is_path_executable()\".\n\nSo you are forcing Windows an extra and unnecessary stat() that only\nis needed on Cygwin, no?\n\n> @@ -1347,7 +1348,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n> \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n> \tCOMPAT_OBJS += compat/mingw.o compat/winansi.o \\\n> \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n> -\t\tcompat/win32/poll.o compat/win32/dirent.o\n> +\t\tcompat/win32/poll.o compat/win32/dirent.o \\\n> +\t\tcompat/win32/executable.o\n\nLooks sensible, even though the filename does not tell what it does\nabout \"executable\". is_executable.o might be a better name for them.\n\n> diff --git a/help.c b/help.c\n> index ebc2c42..d9fae3c 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -106,34 +106,8 @@ static int is_executable(const char *name)\n> \t !S_ISREG(st.st_mode))\n> \t\treturn 0;\n> \n> -#if defined(WIN32) || defined(__CYGWIN__)\n> -\t/* On Windows we cannot use the executable bit. The executable\n> -\t * state is determined by extension only. We do this first\n> -\t * because with virus scanners opening an executeable for\n> -\t * reading is potentially expensive.\n> -\t */\n> -\tif (has_extension(name, \".exe\"))\n> -\t\treturn S_IXUSR;\n> -\n> -#if defined(__CYGWIN__)\n> -if ((st.st_mode & S_IXUSR) == 0)\n> -#endif\n> -{\t/* now that we know it does not have an executable extension,\n> -\t peek into the file instead */\n> -\tchar buf[3] = { 0 };\n> -\tint n;\n> -\tint fd = open(name, O_RDONLY);\n> -\tst.st_mode &= ~S_IXUSR;\n> -\tif (fd >= 0) {\n> -\t\tn = read(fd, buf, 2);\n> -\t\tif (n == 2)\n> -\t\t\t/* look for a she-bang */\n> -\t\t\tif (!strcmp(buf, \"#!\"))\n> -\t\t\t\tst.st_mode |= S_IXUSR;\n> -\t\tclose(fd);\n> -\t}\n> -}\n> -#endif\n> +\tcorrect_executable_stat(name, &st);\n> +\n\nYuck.\n\nWhy should we need even a single line of the implementation of a\nfunction that tells if a given pathname contains an executable\ncommand, which we know is platform specific? \n\nOn Posix systems, the implementation will be \"stat() and check\nS_IXUSR\". On pure Windows, it will be \"check .exe, or open it and\nread the first two bytes\". On Cygwin, it will also be \"check .exe,\nstat() and check S_IXUSR, or open it and read the first two bytes.\n\nIt is not like the caller of is_executable() needed to run stat for\nother purposes on its own and we can optimize by either borrowing\nthe stat data the caller already collected for us, or returning the\nstat data we collected for later use by the caller. The use of stat\nis entirely contained in the POSIX implementation of this function.\n\nWhy are you so dead-set to shoehorn the different semantics into\n\"struct stat\" that we know is not an appropriate abstraction across\nplatforms?\n"},{"id":"197059","messageId":"7v628k54hy.fsf@alter.siamese.dyndns.org","threadId":"31233","inReplyTo":"7v7gt06nyk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T19:39:37Z","receivedAt":"2012-08-15T19:39:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> diff --git a/help.c b/help.c\n>> ...\n>> +\n>\n> Yuck.\n>\n> Why should we need even a single line of the implementation of a\n> function that tells if a given pathname contains an executable\n> command, which we know is platform specific? \n\nSorry; sent without sufficient proofreading. \n\nObviously we need such an implementation (per platform) somewhere in\nthe system.\n\nWhat I meant to ask was \"Why should we need a function whose\nimplementation has to be platform-specific in this help.c file\".\nThe last part of the sentence was missing.\n"},{"id":"197069","messageId":"20120815222911.GA44080@book.hvoigt.net","threadId":"31233","inReplyTo":"7v7gt06nyk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2012-08-15T22:29:13Z","receivedAt":"2012-08-15T22:29:13Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi Junio,\n\nOn Wed, Aug 15, 2012 at 10:53:55AM -0700, Junio C Hamano wrote:\n> Heiko Voigt writes:\n> \n> > On Mon, Aug 13, 2012 at 10:48:14AM -0700, Junio C Hamano wrote:\n> >> Heiko Voigt writes:\n> >> > What do you think?\n> >> \n> >> Does having the \"stat()\" help on Windows in any way? Does it ever\n> >> return an executable bit by itself?\n> >\n> > No, AFAIK it does not return anything about executability. But I think\n> > the stat is still necessary to verify that the file exists and is a\n> > regular file.\n> \n> But if you are going to read it anyway, you can tell it from open()\n> and read() of the first 2 bytes failing, no? That will still be an\n> implementation detail of platform specific \"is_path_executable()\".\n\nNo I am not opening the file anyway. Only when it does not have a\n\".exe\" postfix. That at least was the intention of my previous patch in\nthis thread.\n\n> So you are forcing Windows an extra and unnecessary stat() that only\n> is needed on Cygwin, no?\n\nMy first patch in this thread (which lead to this extraction) is about\navoiding the open because it is possibly very costly on our git binaries\nin case a virus scanner is running.\n\nThe optimized (and correct) check does only look at the given filename\nif it contains a \".exe\" postfix. A directory (although unlikely) could\nalso have such a suffix. Should we then consider that directory to be a\ngit command? AFAICS there is no such check in the earlier codepath.\n\nA stat is much cheaper since it does not open the file. For scripts the\nopen is much cheaper than for our binaries because they are much\nsmaller. For clarification: All the git builtin binaries basically\ncontain the same code. Even though they are hardlinked, the system (or\nthe scanner) does still consider them like individual files and executes\na scan for each of them. That at least seems to be the case on a number\nof systems.\n\n> > @@ -1347,7 +1348,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n> > \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n> > \tCOMPAT_OBJS += compat/mingw.o compat/winansi.o \\\n> > \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n> > -\t\tcompat/win32/poll.o compat/win32/dirent.o\n> > +\t\tcompat/win32/poll.o compat/win32/dirent.o \\\n> > +\t\tcompat/win32/executable.o\n> \n> Looks sensible, even though the filename does not tell what it does\n> about \"executable\". is_executable.o might be a better name for them.\n\nAgreed, I was not sure about the name anyway. is_executable.o sounds\nbetter.\n\n> > diff --git a/help.c b/help.c\n> > index ebc2c42..d9fae3c 100644\n> > --- a/help.c\n> > +++ b/help.c\n> > @@ -106,34 +106,8 @@ static int is_executable(const char *name)\n> > \t !S_ISREG(st.st_mode))\n> > \t\treturn 0;\n> > \n> > -#if defined(WIN32) || defined(__CYGWIN__)\n> > -\t/* On Windows we cannot use the executable bit. The executable\n> > -\t * state is determined by extension only. We do this first\n> > -\t * because with virus scanners opening an executeable for\n> > -\t * reading is potentially expensive.\n> > -\t */\n> > -\tif (has_extension(name, \".exe\"))\n> > -\t\treturn S_IXUSR;\n> > -\n> > -#if defined(__CYGWIN__)\n> > -if ((st.st_mode & S_IXUSR) == 0)\n> > -#endif\n> > -{\t/* now that we know it does not have an executable extension,\n> > -\t peek into the file instead */\n> > -\tchar buf[3] = { 0 };\n> > -\tint n;\n> > -\tint fd = open(name, O_RDONLY);\n> > -\tst.st_mode &= ~S_IXUSR;\n> > -\tif (fd >= 0) {\n> > -\t\tn = read(fd, buf, 2);\n> > -\t\tif (n == 2)\n> > -\t\t\t/* look for a she-bang */\n> > -\t\t\tif (!strcmp(buf, \"#!\"))\n> > -\t\t\t\tst.st_mode |= S_IXUSR;\n> > -\t\tclose(fd);\n> > -\t}\n> > -}\n> > -#endif\n> > +\tcorrect_executable_stat(name, &st);\n> > +\n> \n> Yuck.\n> \n> Why should we need even a single line of the implementation of a\n> function that tells if a given pathname contains an executable\n> command, which we know is platform specific? \n> \n> On Posix systems, the implementation will be \"stat() and check\n> S_IXUSR\". On pure Windows, it will be \"check .exe, or open it and\n> read the first two bytes\". On Cygwin, it will also be \"check .exe,\n> stat() and check S_IXUSR, or open it and read the first two bytes.\n> \n> It is not like the caller of is_executable() needed to run stat for\n> other purposes on its own and we can optimize by either borrowing\n> the stat data the caller already collected for us, or returning the\n> stat data we collected for later use by the caller. The use of stat\n> is entirely contained in the POSIX implementation of this function.\n> \n> Why are you so dead-set to shoehorn the different semantics into\n> \"struct stat\" that we know is not an appropriate abstraction across\n> platforms?\n\nI simply think filling in the correct information into the data\nstructure we use on all platforms is the most transparent approach. We\ncould also call this function\n\n\tcorrect_executable_stat_if_needed_by_platform()\n\nbut I considered that name to be too long. As explained above: To be\nfully correct the stat call is still needed to make sure we are not\nlooking at a directory. list_commands_in_dir() from help.c is iterating\nover all entries in a given directory so if we skip the stat we are not\nsure whether we look at something that is not a regular file. Even\nthough its unlikely that there will be a directory named something.exe\nin the git executable directory I would still like to keep the check in\nthere.\n\nWhen first looking at this I also thought about extending the windows\nstat replacement to do this whole logic. Then we could completely remove\nthis platform specific thing. But I decided against that because when\nthe file does not have a \".exe\" postfix we would have an open call in all\nstats and to my knowledge that is very expensive on Windows.\n\nI do not know why you are against filling that information into \"struct\nstat\". I could rephrase the above into something like this\n\n\tif (platform_is_executeable(name))\n\t\treturn 1;\n\nbut its not that easy to replace to a no op by a macro anymore.\n\nCheers Heiko\n"},{"id":"197070","messageId":"7va9xv4uir.fsf@alter.siamese.dyndns.org","threadId":"31233","inReplyTo":"20120815222911.GA44080@book.hvoigt.net","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T23:15:08Z","receivedAt":"2012-08-15T23:15:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> I do not know why you are against filling that information into \"struct\n> stat\".\n\nBecause it is *WRONG*. Isn't it a good enough reason?\n\nIf the issue you are trying to solve were \"\"\"stat emulation on\nWindows and Cygwin does not give the correct x-bit (and the user\nsometimes has to work it around with \"update-index --chmod\"), and by\nusing other clues the emulation can be improved to give a better\nresult\"\"\", I agree that it would be a good solution to have the\n\"\"\"Does it exist as a regular file and ends with \".exe\"? Otherwise\ndoes it start with \"MZ\" or \"#!\"?\"\"\" heuristics in a helper function\ncorrect_executable_stat(), and to have the implementation of stat\nemulation on these two platforms call that shared helper function.\n\nBut look at the caller again. The problem the caller wants this\nfunction to solve is not \"I want you to stat this file.\" It has a\npathname, and only wants to know if it is an executable file. It\ndoes not care about who owns it, what time it was last touched,\netc., but calling the incomplete stat emulation on Windows will try\nto come up with an answer, and the is_executable() function discards\nmost of it. \n\nIn other words, you are solving a wrong problem with that approach.\n\n\"Run stat() and look at S_IXUSR\" happens to be an implementation\ndetail that is valid only on POSIX systems for a function to answer\nthe question: \"Is this an executable file?\", and in that specific\nimplementation, the answer to that question appears in the S_IXUSR\nbit of st.st_mode. That does not mean the \"struct stat\" is the best\ncontainer for the answer to that question on other platforms. So\nwhy structure your abstraction around the inappropriate data\nstructure? Between the function (is_executable) and its callers,\nthere is only one bit that needs to be passed.\n\nMy preference is to remove \"static int is_executable()\" function\nfrom help.c, have an \"extern int is_executable(const char *)\" that\nhas platform-specific implementation in compat/ layer, and call it\nfrom help.c::list_commands_in_dir() without any #ifdef. In\ngit-compat-util.h, have something like:\n\n\t#ifdef __CYGWIN__\n\t#define is_executable(path) cygwin_is_executable(path)\n\t#else\n # ifdef WIN32\n # define is_executable(path) win32_is_executable(path)\n\t# endif\n\t#endif\n\n #ifndef is_exectutable\n\t#define is_executable(path) posix_is_executable(path)\n\t#endif\n\n extern int is_executable(const char *);\n\nI wouldn't mind seeing the implementation of posix_is_executable()\nin help.c, which will be dead-code on Windows and Cygwin, if that\nmakes linking and Makefile easier.\n"},{"id":"197074","messageId":"7vwr0z387c.fsf@alter.siamese.dyndns.org","threadId":"31233","inReplyTo":"7va9xv4uir.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-16T02:02:31Z","receivedAt":"2012-08-16T02:02:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> My preference is to remove \"static int is_executable()\" function\n> from help.c, have an...\n> ... I wouldn't mind seeing the implementation of posix_is_executable()\n> in help.c, which will be dead-code on Windows and Cygwin, if that\n> makes linking and Makefile easier.\n\nAn outline of such a change might look like this.\n\nAs usual, I am not great at names, so you may want to rename the\nshared one. Also I do not know what headers Cygwin and Windows\nspecific bits would need to include, so you would have to include\nmore than just \"git-compat-util.h\" in the compat/*.c files this\npatch adds.\n\nIt would make sense to move the \"does it end with .exe\" check from\nthe caller to the shared function, but because that does not match\nthe concept of the name I came up with (peek-contents), I left it\nout of the function. If there is a good name to express what such a\ncombined \"ends with .exe, or inspected contents look like an\nexecutable\", it would make sense to rename it as such and move\n\".exe\" check to it.\n\nThanks.\n\n Makefile | 6 ++++--\n compat/cygwin.c | 16 ++++++++++++++++\n compat/win32/is_executable.c | 14 ++++++++++++++\n compat/win32/peek_contents.c | 16 ++++++++++++++++\n git-compat-util.h | 9 +++++++++\n help.c | 32 +-------------------------------\n is_executable.c | 12 ++++++++++++\n 7 files changed, 72 insertions(+), 33 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 6b0c961..2bcb9f4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -734,6 +734,7 @@ LIB_OBJS += hash.o\n LIB_OBJS += help.o\n LIB_OBJS += hex.o\n LIB_OBJS += ident.o\n+LIB_OBJS += is_executable.o\n LIB_OBJS += kwset.o\n LIB_OBJS += levenshtein.o\n LIB_OBJS += list-objects.o\n@@ -1073,7 +1074,7 @@ ifeq ($(uname_O),Cygwin)\n \t# Try commenting this out if you suspect MMAP is more efficient\n \tNO_MMAP = YesPlease\n \tX = .exe\n-\tCOMPAT_OBJS += compat/cygwin.o\n+\tCOMPAT_OBJS += compat/cygwin.o compat/win32/peek_contents.o\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n \tSPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield\n endif\n@@ -1257,7 +1258,8 @@ ifeq ($(uname_S),Windows)\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n \tCOMPAT_OBJS = compat/msvc.o compat/winansi.o \\\n \t\tcompat/win32/pthread.o compat/win32/syslog.o \\\n-\t\tcompat/win32/poll.o compat/win32/dirent.o\n+\t\tcompat/win32/poll.o compat/win32/dirent.o \\\n+\t\tcompat/win32/is_executable.o compat/win32/peek_contents.o\n \tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex dfe9b30..5eb948a 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -140,3 +140,19 @@ static int cygwin_lstat_stub(const char *file_name, struct stat *buf)\n stat_fn_t cygwin_stat_fn = cygwin_stat_stub;\n stat_fn_t cygwin_lstat_fn = cygwin_lstat_stub;\n \n+extern int win32_peek_contents_executable(const char *);\n+\n+int cygwin_is_path_executable(const char *name)\n+{\n+\tstruct stat st;\n+\n+\t/* stat, not lstat */\n+\tif (stat(name, &st) || !S_ISREG(st.st_mode))\n+\t\treturn 0;\n+\tif (st.st_mode & S_IXUSR)\n+\t\treturn 1;\n+\n+\tif (has_extension(name, \".exe\"))\n+\t\treturn 1;\n+\treturn win32_peek_contents_executable(name);\n+}\ndiff --git a/compat/win32/is_executable.c b/compat/win32/is_executable.c\nnew file mode 100644\nindex 0000000..f7907b3\n--- /dev/null\n+++ b/compat/win32/is_executable.c\n@@ -0,0 +1,14 @@\n+#include \"git-compat-util.h\"\n+\n+int win32_path_is_executable(const char *name)\n+{\n+\t/*\n+\t * Do whatever you would do on win32 to make sure\n+\t * \"name\" exists here\n+\t */\n+\tif (!file_exists(name))\n+\t\treturn 0;\n+\tif (has_extension(name, \".exe\"))\n+\t\treturn 1;\n+\treturn win32_peek_contents_executable(name);\n+}\ndiff --git a/compat/win32/peek_contents.c b/compat/win32/peek_contents.c\nnew file mode 100644\nindex 0000000..5e425aa\n--- /dev/null\n+++ b/compat/win32/peek_contents.c\n@@ -0,0 +1,16 @@\n+#include \"git-compat-util.h\"\n+\n+int win32_peek_contents_executable(const char *name)\n+{\n+\tchar buf[2];\n+\tint n, fd;\n+\n+\tfd = open(name, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn 0;\n+\tn = read(fd, buf, 2);\n+\tclose(fd);\n+\treturn (n == 2 &&\n+\t\t/* DOS executables start with \"MZ\" */\n+\t\t(!memcmp(buf, \"#!\", 2) || !memcmp(buf, \"MZ\", 2)));\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 35b095e..e8f8cb1 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -607,4 +607,13 @@ int remove_or_warn(unsigned int mode, const char *path);\n /* Get the passwd entry for the UID of the current process. */\n struct passwd *xgetpwuid_self(void);\n \n+/* Does the given path name an executable command? */\n+#if defined(__CYGWIN__)\n+#define is_path_executable(p) cygwin_is_path_executable(p)\n+#elif defined(WIN32)\n+#define is_path_executable(p) win32_is_path_executable(p)\n+#endif\n+\n+extern int is_path_executable(const char *);\n+\n #endif\ndiff --git a/help.c b/help.c\nindex 2a42ec6..672d9d3 100644\n--- a/help.c\n+++ b/help.c\n@@ -98,36 +98,6 @@ static void pretty_print_string_list(struct cmdnames *cmds,\n \tstring_list_clear(&list, 0);\n }\n \n-static int is_executable(const char *name)\n-{\n-\tstruct stat st;\n-\n-\tif (stat(name, &st) || /* stat, not lstat */\n-\t !S_ISREG(st.st_mode))\n-\t\treturn 0;\n-\n-#if defined(WIN32) || defined(__CYGWIN__)\n-#if defined(__CYGWIN__)\n-if ((st.st_mode & S_IXUSR) == 0)\n-#endif\n-{\t/* cannot trust the executable bit, peek into the file instead */\n-\tchar buf[3] = { 0 };\n-\tint n;\n-\tint fd = open(name, O_RDONLY);\n-\tst.st_mode &= ~S_IXUSR;\n-\tif (fd >= 0) {\n-\t\tn = read(fd, buf, 2);\n-\t\tif (n == 2)\n-\t\t\t/* DOS executables start with \"MZ\" */\n-\t\t\tif (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n-\t\t\t\tst.st_mode |= S_IXUSR;\n-\t\tclose(fd);\n-\t}\n-}\n-#endif\n-\treturn st.st_mode & S_IXUSR;\n-}\n-\n static void list_commands_in_dir(struct cmdnames *cmds,\n \t\t\t\t\t const char *path,\n \t\t\t\t\t const char *prefix)\n@@ -155,7 +125,7 @@ static void list_commands_in_dir(struct cmdnames *cmds,\n \n \t\tstrbuf_setlen(&buf, len);\n \t\tstrbuf_addstr(&buf, de->d_name);\n-\t\tif (!is_executable(buf.buf))\n+\t\tif (!is_path_executable(buf.buf))\n \t\t\tcontinue;\n \n \t\tentlen = strlen(de->d_name) - prefix_len;\ndiff --git a/is_executable.c b/is_executable.c\nnew file mode 100644\nindex 0000000..9dacd2a\n--- /dev/null\n+++ b/is_executable.c\n@@ -0,0 +1,12 @@\n+#include \"git-compat-util.h\"\n+\n+int is_path_executable(const char *name)\n+{\n+\tstruct stat st;\n+\n+\t/* stat, not lstat */\n+\tif (stat(name, &st) || !S_ISREG(st.st_mode))\n+\t\treturn 0;\n+\n+\treturn st.st_mode & S_IXUSR;\n+}\n"},{"id":"197110","messageId":"20120816163543.GA45275@book.hvoigt.net","threadId":"31233","inReplyTo":"7vwr0z387c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] help: correct behavior for is_executable on Windows","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2012-08-16T16:35:43Z","receivedAt":"2012-08-16T16:35:43Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi Junio,\n\nOn Wed, Aug 15, 2012 at 07:02:31PM -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > My preference is to remove \"static int is_executable()\" function\n> > from help.c, have an...\n> > ... I wouldn't mind seeing the implementation of posix_is_executable()\n> > in help.c, which will be dead-code on Windows and Cygwin, if that\n> > makes linking and Makefile easier.\n> \n> An outline of such a change might look like this.\n\nThanks for the outline and the clarification, I will use it to cook up\nsomething.\n\nCheers Heiko\n"}]}