From cc5e1bf992470e0d514c743e75a0325c8b592f38 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 21 Apr 2018 13:14:08 +0200 Subject: [PATCH 1/3] gettext: avoid initialization if the locale dir is not present The runtime of a simple `git.exe version` call on Windows is currently dominated by the gettext setup, adding a whopping ~150ms to the ~210ms total. Given that this cost is added to each and every git.exe invocation goes through common-main's invocation of git_setup_gettext(), and given that scripts have to call git.exe dozens, if not hundreds, of times, this is a substantial performance penalty. This is particularly pointless when considering that Git for Windows ships without localization (to keep the installer's size to a bearable ~34MB): all that time setting up gettext is for naught. To be clear, Git for Windows *needs* to be compiled with localization, for the following reasons: - to allow users to copy add-on localization in case they want it, and - to fix the nasty error message BUG: your vsnprintf is broken (returned -1) by using libgettext's override of vsnprintf() that does not share the behavior of msvcrt.dll's version of vsnprintf(). So let's be smart about it and skip setting up gettext if the locale directory is not even present. Since localization might be missing for not-yet-supported locales, this will not break anything. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- gettext.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gettext.c b/gettext.c index 6b64d5c2e8..3eb20c5f95 100644 --- a/gettext.c +++ b/gettext.c @@ -163,6 +163,9 @@ void git_setup_gettext(void) if (!podir) podir = system_path(GIT_LOCALE_PATH); + if (!is_directory(podir)) + return; + bindtextdomain("git", podir); setlocale(LC_MESSAGES, ""); setlocale(LC_TIME, ""); From 0210231b0803f881bf765bb2d7284c119c6ce9b6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 21 Apr 2018 13:14:28 +0200 Subject: [PATCH 2/3] git_setup_gettext: plug memory leak The system_path() function returns a freshly-allocated string. We need to release it. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- gettext.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/gettext.c b/gettext.c index 3eb20c5f95..4f59dfa3d1 100644 --- a/gettext.c +++ b/gettext.c @@ -159,18 +159,23 @@ static void init_gettext_charset(const char *domain) void git_setup_gettext(void) { const char *podir = getenv(GIT_TEXT_DOMAIN_DIR_ENVIRONMENT); + char *p = NULL; if (!podir) - podir = system_path(GIT_LOCALE_PATH); + podir = p = system_path(GIT_LOCALE_PATH); - if (!is_directory(podir)) + if (!is_directory(podir)) { + free(p); return; + } bindtextdomain("git", podir); setlocale(LC_MESSAGES, ""); setlocale(LC_TIME, ""); init_gettext_charset("git"); textdomain("git"); + + free(p); } /* return the number of columns of string 's' in current locale */ From 4d5b4c247597a5891beb84d933b8eecb77c76186 Mon Sep 17 00:00:00 2001 From: Philip Oakley Date: Sat, 21 Apr 2018 13:18:42 +0200 Subject: [PATCH 3/3] Avoid multiple PREFIX definitions The short and sweet PREFIX can be confused when used in many places. Rename both usages to better describe their purpose. EXEC_CMD_PREFIX is used in full to disambiguate it from the nearby GIT_EXEC_PATH. The PREFIX in sideband.c, while nominally independant of the exec_cmd PREFIX, does reside within libgit[1], so the definitions would clash when taken together with a PREFIX given on the command line for use by exec_cmd.c. Noticed when compiling Git for Windows using MSVC/Visual Studio [1] which reports the conflict beteeen the command line definition and the definition in sideband.c within the libgit project. [1] the libgit functions are brought into a single sub-project within the Visual Studio construction script provided in contrib, and hence uses a single command for both exec_cmd.c and sideband.c. Signed-off-by: Philip Oakley Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Makefile | 2 +- exec_cmd.c | 4 ++-- sideband.c | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 154929f1c8..068a22f40b 100644 --- a/Makefile +++ b/Makefile @@ -2261,7 +2261,7 @@ exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ - '-DPREFIX="$(prefix_SQ)"' + '-DFALLBACK_RUNTIME_PREFIX="$(prefix_SQ)"' builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \ diff --git a/exec_cmd.c b/exec_cmd.c index 6e114f8b3a..ff5a297591 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -48,7 +48,7 @@ static const char *system_prefix(void) !(prefix = strip_path_suffix(executable_dirname, GIT_EXEC_PATH)) && !(prefix = strip_path_suffix(executable_dirname, BINDIR)) && !(prefix = strip_path_suffix(executable_dirname, "git"))) { - prefix = PREFIX; + prefix = FALLBACK_RUNTIME_PREFIX; trace_printf("RUNTIME_PREFIX requested, " "but prefix computation failed. " "Using static fallback '%s'.\n", prefix); @@ -243,7 +243,7 @@ void git_resolve_executable_dir(const char *argv0) */ static const char *system_prefix(void) { - return PREFIX; + return FALLBACK_RUNTIME_PREFIX; } /* diff --git a/sideband.c b/sideband.c index 6d7f943e43..325bf0e974 100644 --- a/sideband.c +++ b/sideband.c @@ -13,7 +13,7 @@ * the remote died unexpectedly. A flush() concludes the stream. */ -#define PREFIX "remote: " +#define DISPLAY_PREFIX "remote: " #define ANSI_SUFFIX "\033[K" #define DUMB_SUFFIX " " @@ -49,7 +49,7 @@ int recv_sideband(const char *me, int in_stream, int out) switch (band) { case 3: strbuf_addf(&outbuf, "%s%s%s", outbuf.len ? "\n" : "", - PREFIX, buf + 1); + DISPLAY_PREFIX, buf + 1); retval = SIDEBAND_REMOTE_ERROR; break; case 2: @@ -67,7 +67,7 @@ int recv_sideband(const char *me, int in_stream, int out) int linelen = brk - b; if (!outbuf.len) - strbuf_addstr(&outbuf, PREFIX); + strbuf_addstr(&outbuf, DISPLAY_PREFIX); if (linelen > 0) { strbuf_addf(&outbuf, "%.*s%s%c", linelen, b, suffix, *brk); @@ -81,8 +81,8 @@ int recv_sideband(const char *me, int in_stream, int out) } if (*b) - strbuf_addf(&outbuf, "%s%s", - outbuf.len ? "" : PREFIX, b); + strbuf_addf(&outbuf, "%s%s", outbuf.len ? + "" : DISPLAY_PREFIX, b); break; case 1: write_or_die(out, buf + 1, len);