Re: [PATCH v7 1/2] exec_cmd: expose is_executable function
From: Jonathan Nieder <hidden>
Date: 2017-04-25 18:05:00
Hi, Brandon Williams wrote:
Move the logic for 'is_executable()' from help.c to exec_cmd.c and expose it so that callers from outside help.c can access the function. This is to enable run-command to be able to query if a file is executable in a future patch. Signed-off-by: Brandon Williams <redacted> --- exec_cmd.c | 42 ++++++++++++++++++++++++++++++++++++++++++ exec_cmd.h | 1 + help.c | 42 ------------------------------------------ 3 files changed, 43 insertions(+), 42 deletions(-)
Makes sense. Seems like as fine place for it. (exec_cmd.c is mostly an implementation detail of run_command, to hold the logic for executing a git command. This is another run_command helper, and it doesn't feel illogical to find it there. Another alternative place to put t would be in run-command.c.)
quoted hunk ↗ jump to hunk
diff --git a/exec_cmd.c b/exec_cmd.c index fb94aeba9..6d9481e26 100644 --- a/exec_cmd.c +++ b/exec_cmd.c@@ -149,3 +149,45 @@ int execl_git_cmd(const char *cmd,...) argv[argc] = NULL; return execv_git_cmd(argv); } + +int is_executable(const char *name)
nit: it's a good practice to find a logical place for a new function in the existing file, instead of defaulting to the end. That way, the file is easier to read sequentially, and if two different patches want to add a function to the same file then they are less likely to conflict. This could go before prepare_git_cmd, for example. With or without such a tweak, Reviewed-by: Jonathan Nieder <redacted>
diff --git i/exec_cmd.c w/exec_cmd.c
index 6d9481e26d..601fbc43bc 100644
--- i/exec_cmd.c
+++ w/exec_cmd.c@@ -104,6 +104,48 @@ void setup_path(void) strbuf_release(&new_path); } +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(GIT_WINDOWS_NATIVE) + /* + * On Windows there is no executable bit. The file extension + * indicates whether it can be run as an executable, and Git + * has special-handling to detect scripts and launch them + * through the indicated script interpreter. We test for the + * file extension first because virus scanners may make + * it quite expensive to open many files. + */ + if (ends_with(name, ".exe")) + return S_IXUSR; + +{ + /* + * Now that we know it does not have an executable extension, + * 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) + /* look for a she-bang */ + if (!strcmp(buf, "#!")) + st.st_mode |= S_IXUSR; + close(fd); + } +} +#endif + return st.st_mode & S_IXUSR; +} + const char **prepare_git_cmd(struct argv_array *out, const char **argv) { argv_array_push(out, "git");
@@ -149,45 +191,3 @@ int execl_git_cmd(const char *cmd,...) argv[argc] = NULL; return execv_git_cmd(argv); } - -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(GIT_WINDOWS_NATIVE) - /* - * On Windows there is no executable bit. The file extension - * indicates whether it can be run as an executable, and Git - * has special-handling to detect scripts and launch them - * through the indicated script interpreter. We test for the - * file extension first because virus scanners may make - * it quite expensive to open many files. - */ - if (ends_with(name, ".exe")) - return S_IXUSR; - -{ - /* - * Now that we know it does not have an executable extension, - * 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) - /* look for a she-bang */ - if (!strcmp(buf, "#!")) - st.st_mode |= S_IXUSR; - close(fd); - } -} -#endif - return st.st_mode & S_IXUSR; -}
diff --git i/exec_cmd.h w/exec_cmd.h
index 48dd18a0d4..5e8200b952 100644
--- i/exec_cmd.h
+++ w/exec_cmd.h@@ -7,11 +7,11 @@ extern void git_set_argv_exec_path(const char *exec_path); extern void git_extract_argv0_path(const char *path); extern const char *git_exec_path(void); extern void setup_path(void); +extern int is_executable(const char *name); extern const char **prepare_git_cmd(struct argv_array *out, const char **argv); extern int execv_git_cmd(const char **argv); /* NULL terminated */ LAST_ARG_MUST_BE_NULL extern int execl_git_cmd(const char *cmd, ...); extern char *system_path(const char *path); -extern int is_executable(const char *name); #endif /* GIT_EXEC_CMD_H */