git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] help: correct behavior for is_executable on Windows

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 16, 2012, 02:02 UTC
Message-ID
<7vwr0z387c.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7va9xv4uir.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 5 quoted lines
> My preference is to remove "static int is_executable()" function
> from help.c, have an...
> ... I wouldn't mind seeing the implementation of posix_is_executable()
> in help.c, which will be dead-code on Windows and Cygwin, if that
> makes linking and Makefile easier.
An outline of such a change might look like this.

As usual, I am not great at names, so you may want to rename the shared one. Also I do not know what headers Cygwin and Windows specific bits would need to include, so you would have to include more than just "git-compat-util.h" in the compat/*.c files this patch adds.

It would make sense to move the "does it end with .exe" check from the caller to the shared function, but because that does not match the concept of the name I came up with (peek-contents), I left it out of the function. If there is a good name to express what such a combined "ends with .exe, or inspected contents look like an executable", it would make sense to rename it as such and move ".exe" check to it.

Thanks.
 Makefile                     |  6 ++++--
 compat/cygwin.c              | 16 ++++++++++++++++
 compat/win32/is_executable.c | 14 ++++++++++++++
 compat/win32/peek_contents.c | 16 ++++++++++++++++
 git-compat-util.h            |  9 +++++++++
 help.c                       | 32 +-------------------------------
 is_executable.c              | 12 ++++++++++++
 7 files changed, 72 insertions(+), 33 deletions(-)
diff --git a/Makefile b/Makefile
index 6b0c961..2bcb9f4 100644
--- a/Makefile
+++ b/Makefile
@@ -734,6 +734,7 @@ LIB_OBJS += hash.o
 LIB_OBJS += help.o
 LIB_OBJS += hex.o
 LIB_OBJS += ident.o
+LIB_OBJS += is_executable.o
 LIB_OBJS += kwset.o
 LIB_OBJS += levenshtein.o
 LIB_OBJS += list-objects.o
@@ -1073,7 +1074,7 @@ ifeq ($(uname_O),Cygwin)
 	# Try commenting this out if you suspect MMAP is more efficient
 	NO_MMAP = YesPlease
 	X = .exe
-	COMPAT_OBJS += compat/cygwin.o
+	COMPAT_OBJS += compat/cygwin.o compat/win32/peek_contents.o
 	UNRELIABLE_FSTAT = UnfortunatelyYes
 	SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield
 endif
@@ -1257,7 +1258,8 @@ ifeq ($(uname_S),Windows)
 	BASIC_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
 	COMPAT_OBJS = compat/msvc.o compat/winansi.o \
 		compat/win32/pthread.o compat/win32/syslog.o \
-		compat/win32/poll.o compat/win32/dirent.o
+		compat/win32/poll.o compat/win32/dirent.o \
+		compat/win32/is_executable.o compat/win32/peek_contents.o
 	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
 	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
 	EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
diff --git a/compat/cygwin.c b/compat/cygwin.c
index dfe9b30..5eb948a 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -140,3 +140,19 @@ static int cygwin_lstat_stub(const char *file_name, struct stat *buf)
 stat_fn_t cygwin_stat_fn = cygwin_stat_stub;
 stat_fn_t cygwin_lstat_fn = cygwin_lstat_stub;
 
+extern int win32_peek_contents_executable(const char *);
+
+int cygwin_is_path_executable(const char *name)
+{
+	struct stat st;
+
+	/* stat, not lstat */
+	if (stat(name, &st) || !S_ISREG(st.st_mode))
+		return 0;
+	if (st.st_mode & S_IXUSR)
+		return 1;
+
+	if (has_extension(name, ".exe"))
+		return 1;
+	return win32_peek_contents_executable(name);
+}
diff --git a/compat/win32/is_executable.c b/compat/win32/is_executable.c
new file mode 100644
index 0000000..f7907b3
--- /dev/null
+++ b/compat/win32/is_executable.c
@@ -0,0 +1,14 @@
+#include "git-compat-util.h"
+
+int win32_path_is_executable(const char *name)
+{
+	/*
+	 * Do whatever you would do on win32 to make sure
+	 * "name" exists here
+	 */
+	if (!file_exists(name))
+		return 0;
+	if (has_extension(name, ".exe"))
+		return 1;
+	return win32_peek_contents_executable(name);
+}
diff --git a/compat/win32/peek_contents.c b/compat/win32/peek_contents.c
new file mode 100644
index 0000000..5e425aa
--- /dev/null
+++ b/compat/win32/peek_contents.c
@@ -0,0 +1,16 @@
+#include "git-compat-util.h"
+
+int win32_peek_contents_executable(const char *name)
+{
+	char buf[2];
+	int n, fd;
+
+	fd = open(name, O_RDONLY);
+	if (fd < 0)
+		return 0;
+	n = read(fd, buf, 2);
+	close(fd);
+	return (n == 2 &&
+		/* DOS executables start with "MZ" */
+		(!memcmp(buf, "#!", 2) || !memcmp(buf, "MZ", 2)));
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 35b095e..e8f8cb1 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -607,4 +607,13 @@ int remove_or_warn(unsigned int mode, const char *path);
 /* Get the passwd entry for the UID of the current process. */
 struct passwd *xgetpwuid_self(void);
 
+/* Does the given path name an executable command? */
+#if defined(__CYGWIN__)
+#define is_path_executable(p) cygwin_is_path_executable(p)
+#elif defined(WIN32)
+#define is_path_executable(p) win32_is_path_executable(p)
+#endif
+
+extern int is_path_executable(const char *);
+
 #endif
diff --git a/help.c b/help.c
index 2a42ec6..672d9d3 100644
--- a/help.c
+++ b/help.c
@@ -98,36 +98,6 @@ static void pretty_print_string_list(struct cmdnames *cmds,
 	string_list_clear(&list, 0);
 }
 
-static int is_executable(const char *name)
-{
-	struct stat st;
-
-	if (stat(name, &st) || /* stat, not lstat */
-	    !S_ISREG(st.st_mode))
-		return 0;
-
-#if defined(WIN32) || defined(__CYGWIN__)
-#if defined(__CYGWIN__)
-if ((st.st_mode & S_IXUSR) == 0)
-#endif
-{	/* cannot trust the executable bit, peek into the file instead */
-	char buf[3] = { 0 };
-	int n;
-	int fd = open(name, O_RDONLY);
-	st.st_mode &= ~S_IXUSR;
-	if (fd >= 0) {
-		n = read(fd, buf, 2);
-		if (n == 2)
-			/* DOS executables start with "MZ" */
-			if (!strcmp(buf, "#!") || !strcmp(buf, "MZ"))
-				st.st_mode |= S_IXUSR;
-		close(fd);
-	}
-}
-#endif
-	return st.st_mode & S_IXUSR;
-}
-
 static void list_commands_in_dir(struct cmdnames *cmds,
 					 const char *path,
 					 const char *prefix)
@@ -155,7 +125,7 @@ static void list_commands_in_dir(struct cmdnames *cmds,
 
 		strbuf_setlen(&buf, len);
 		strbuf_addstr(&buf, de->d_name);
-		if (!is_executable(buf.buf))
+		if (!is_path_executable(buf.buf))
 			continue;
 
 		entlen = strlen(de->d_name) - prefix_len;
diff --git a/is_executable.c b/is_executable.c
new file mode 100644
index 0000000..9dacd2a
--- /dev/null
+++ b/is_executable.c
@@ -0,0 +1,12 @@
+#include "git-compat-util.h"
+
+int is_path_executable(const char *name)
+{
+	struct stat st;
+
+	/* stat, not lstat */
+	if (stat(name, &st) || !S_ISREG(st.st_mode))
+		return 0;
+
+	return st.st_mode & S_IXUSR;
+}
Previous: Junio C HamanoNext: Heiko Voigt
Message 10 of 11 in “help: correct behavior for is_executable on Windows”
  1. help: correct behavior for is_executable on WindowsHeiko Voigt, Aug 11, 2012
  2. Junio C HamanoAug 12, 2012
  3. Heiko VoigtAug 13, 2012
  4. Junio C HamanoAug 13, 2012
  5. Heiko VoigtAug 15, 2012
  6. Junio C HamanoAug 15, 2012
  7. Junio C HamanoAug 15, 2012
  8. Heiko VoigtAug 15, 2012
  9. Junio C HamanoAug 15, 2012
  10. Junio C HamanoAug 16, 2012
  11. Heiko VoigtAug 16, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.