The way the code is structured now, we have to repeat
ourselves in fetching the environment variables (which will
get annoying as we add more).  Instead, let's use an
argv_array.  That removes a lot of the extra conditionals
and makes it easier to add new variables.

It does means we'll leak the memory for the array, but:

  1. This function is only called once per program.

  2. We're now leaking heap memory instead of wasting BSS on
     the static array.

Signed-off-by: Jeff King <p...@peff.net>
---
I actually think we can free pager_process.env after
start_command is run. You _cannot_ do so with argv, though,
and I'd rather leak this tiny bit than have a hard-to-track
assumption on memory lifetime buried here.

 pager.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/pager.c b/pager.c
index 0cc75a8..90d237e 100644
--- a/pager.c
+++ b/pager.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include "argv-array.h"
 
 #ifndef DEFAULT_PAGER
 #define DEFAULT_PAGER "less"
@@ -63,6 +64,7 @@ const char *git_pager(int stdout_is_tty)
 void setup_pager(void)
 {
        const char *pager = git_pager(isatty(1));
+       struct argv_array env = ARGV_ARRAY_INIT;
 
        if (!pager || pager_in_use())
                return;
@@ -80,17 +82,13 @@ void setup_pager(void)
        pager_process.use_shell = 1;
        pager_process.argv = pager_argv;
        pager_process.in = -1;
-       if (!getenv("LESS") || !getenv("LV")) {
-               static const char *env[3];
-               int i = 0;
-
-               if (!getenv("LESS"))
-                       env[i++] = "LESS=FRSX";
-               if (!getenv("LV"))
-                       env[i++] = "LV=-c";
-               env[i] = NULL;
-               pager_process.env = env;
-       }
+
+       if (!getenv("LESS"))
+               argv_array_push(&env, "LESS=FRSX");
+       if (!getenv("LV"))
+               argv_array_push(&env, "LV=-c");
+       pager_process.env = argv_array_detach(&env, NULL);
+
        if (start_command(&pager_process))
                return;
 
-- 
1.8.5.2.500.g8060133

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to