From 39b2f6af6e009c2c087eeec293e98222eb396263 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 6 Sep 2017 08:30:28 -0400 Subject: [PATCH 1/2] system_path: move RUNTIME_PREFIX to a sub-function The system_path() function has an #ifdef in the middle of it. Let's move the conditional logic into a sub-function. This isolates it more, which will make it easier to change and add to. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- exec_cmd.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index fb94aeba9c..61092e9715 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -7,19 +7,12 @@ static const char *argv_exec_path; static const char *argv0_path; -char *system_path(const char *path) +#ifdef RUNTIME_PREFIX + +static const char *system_prefix(void) { -#ifdef RUNTIME_PREFIX static const char *prefix; -#else - static const char *prefix = PREFIX; -#endif - struct strbuf d = STRBUF_INIT; - if (is_absolute_path(path)) - return xstrdup(path); - -#ifdef RUNTIME_PREFIX assert(argv0_path); assert(is_absolute_path(argv0_path)); @@ -32,9 +25,25 @@ char *system_path(const char *path) "but prefix computation failed. " "Using static fallback '%s'.\n", prefix); } -#endif + return prefix; +} +#else - strbuf_addf(&d, "%s/%s", prefix, path); +static const char *system_prefix(void) +{ + return PREFIX; +} + +#endif /* RUNTIME_PREFIX */ + +char *system_path(const char *path) +{ + struct strbuf d = STRBUF_INIT; + + if (is_absolute_path(path)) + return xstrdup(path); + + strbuf_addf(&d, "%s/%s", system_prefix(), path); return strbuf_detach(&d, NULL); } From c1bb33c99c3a35012f7242b1d83b04233057555c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 6 Sep 2017 08:32:10 -0400 Subject: [PATCH 2/2] git_extract_argv0_path: do nothing without RUNTIME_PREFIX When the RUNTIME_PREFIX compile-time knob isn't set, we never look at the argv0_path we extract. We can push its declaration inside the #ifdef to make it more clear that the extract code is effectively a noop. This also un-confuses leak-checking of the argv0_path variable when RUNTIME_PREFIX isn't set. The compiler is free to drop this static variable that we set but never look at (and "gcc -O2" does so). But the compiler still must call strbuf_detach(), since it doesn't know whether that function has side effects; it just throws away the result rather than putting it into the global. Leak-checkers which work by scanning the data segment for pointers to heap blocks would normally consider the block as reachable at program end. But if the compiler removes the variable entirely, there's nothing to find. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- exec_cmd.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index 61092e9715..ce192a2d64 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -5,9 +5,9 @@ #define MAX_ARGS 32 static const char *argv_exec_path; -static const char *argv0_path; #ifdef RUNTIME_PREFIX +static const char *argv0_path; static const char *system_prefix(void) { @@ -27,6 +27,20 @@ static const char *system_prefix(void) } return prefix; } + +void git_extract_argv0_path(const char *argv0) +{ + const char *slash; + + if (!argv0 || !*argv0) + return; + + slash = find_last_dir_sep(argv0); + + if (slash) + argv0_path = xstrndup(argv0, slash - argv0); +} + #else static const char *system_prefix(void) @@ -34,6 +48,10 @@ static const char *system_prefix(void) return PREFIX; } +void git_extract_argv0_path(const char *argv0) +{ +} + #endif /* RUNTIME_PREFIX */ char *system_path(const char *path) @@ -47,19 +65,6 @@ char *system_path(const char *path) return strbuf_detach(&d, NULL); } -void git_extract_argv0_path(const char *argv0) -{ - const char *slash; - - if (!argv0 || !*argv0) - return; - - slash = find_last_dir_sep(argv0); - - if (slash) - argv0_path = xstrndup(argv0, slash - argv0); -} - void git_set_argv_exec_path(const char *exec_path) { argv_exec_path = exec_path;