[PATCH 1/3] setup_pager: refactor LESS/LV environment setting

From

Jeff King <peff@peff.net>

Date

Jan 17, 2014, 04:21 UTC

Message-ID

<20140117042101.GA23443@sigill.intra.peff.net>

In-Reply-To

<20140117041430.GB19551@sigill.intra.peff.net>

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 <peff@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