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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <peff@xxxxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]