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 <[email protected]>
---
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
