system_path(): always return free'able memory to the caller
The function sometimes returns a newly allocated string and sometimes returns a borrowed string, the latter of which the callers must not free(). The existing callers all assume that the return value belongs to the callee and most of them copy it with strdup() when they want to keep it around. They end up leaking the returned copy when the callee returned a new string because they cannot tell if they should free it. Change the contract between the callers and system_path() to make the returned string owned by the callers; they are responsible for freeing it when done, but they do not have to make their own copy to store it away. Adjust the callers to make sure they do not leak the returned string once they are done, but do not bother freeing it just before dying, exiting or exec'ing other program to avoid unnecessary churn. Reported-by: Alexander Kuleshov <kuleshovmail@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Nov 24, 2014 at 11:33 UTC
59362e560d3c439e77768983b00eade08be9bc3e
4 files changed
+21
-12
builtin/help.c
+7
-2
@@ -322,16 +322,18 @@ static void setup_man_path(void)
322
{
323
struct strbuf new_path = STRBUF_INIT;
324
const char *old_path = getenv("MANPATH");
325
+ char *git_man_path = system_path(GIT_MAN_PATH);
326
327
/* We should always put ':' after our path. If there is no
328
* old_path, the ':' at the end will let 'man' to try
329
* system-wide paths after ours to find the manual page. If
330
* there is old_path, we need ':' as delimiter. */
330
- strbuf_addstr(&new_path, system_path(GIT_MAN_PATH));
331
+ strbuf_addstr(&new_path, git_man_path);
332
strbuf_addch(&new_path, ':');
333
if (old_path)
334
strbuf_addstr(&new_path, old_path);
335
336
+ free(git_man_path);
337
setenv("MANPATH", new_path.buf, 1);
338
339
strbuf_release(&new_path);
@@ -381,8 +383,10 @@ static void show_info_page(const char *git_cmd)
383
static void get_html_page_path(struct strbuf *page_path, const char *page)
384
{
385
struct stat st;
386
+ char *to_free = NULL;
387
+
388
if (!html_path)
385
- html_path = system_path(GIT_HTML_PATH);
389
+ html_path = to_free = system_path(GIT_HTML_PATH);
390
391
/* Check that we have a git documentation directory. */
392
if (!strstr(html_path, "://")) {
@@ -393,6 +397,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
397
398
strbuf_init(page_path, 0);
399
strbuf_addf(page_path, "%s/%s.html", html_path, page);
400
+ free(to_free);
401
}
402
403
/*
builtin/init-db.c
+10
-5
@@ -119,15 +119,18 @@ static void copy_templates(const char *template_dir)
119
DIR *dir;
120
const char *git_dir = get_git_dir();
121
int len = strlen(git_dir);
122
+ char *to_free = NULL;
123
124
if (!template_dir)
125
template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
126
if (!template_dir)
127
template_dir = init_db_template_dir;
128
if (!template_dir)
128
- template_dir = system_path(DEFAULT_GIT_TEMPLATE_DIR);
129
- if (!template_dir[0])
129
+ template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
130
+ if (!template_dir[0]) {
131
+ free(to_free);
132
return;
133
+ }
134
template_len = strlen(template_dir);
135
if (PATH_MAX <= (template_len+strlen("/config")))
136
die(_("insanely long template path %s"), template_dir);
@@ -139,7 +142,7 @@ static void copy_templates(const char *template_dir)
142
dir = opendir(template_path);
143
if (!dir) {
144
warning(_("templates not found %s"), template_dir);
142
- return;
145
+ goto free_return;
146
}
147
148
/* Make sure that template is from the correct vintage */
@@ -155,8 +158,7 @@ static void copy_templates(const char *template_dir)
158
"a wrong format version %d from '%s'"),
159
repository_format_version,
160
template_dir);
158
- closedir(dir);
159
- return;
161
+ goto close_free_return;
162
}
163
164
memcpy(path, git_dir, len);
@@ -166,7 +168,10 @@ static void copy_templates(const char *template_dir)
168
copy_templates_1(path, len,
169
template_path, template_len,
170
dir);
171
+close_free_return:
172
closedir(dir);
173
+free_return:
174
+ free(to_free);
175
}
176
177
static int git_init_db_config(const char *k, const char *v, void *cb)
exec_cmd.c
+3
-4
@@ -6,7 +6,7 @@
6
static const char *argv_exec_path;
7
static const char *argv0_path;
8
9
-const char *system_path(const char *path)
9
+char *system_path(const char *path)
10
{
11
#ifdef RUNTIME_PREFIX
12
static const char *prefix;
@@ -16,7 +16,7 @@ const char *system_path(const char *path)
16
struct strbuf d = STRBUF_INIT;
17
18
if (is_absolute_path(path))
19
- return path;
19
+ return xstrdup(path);
20
21
#ifdef RUNTIME_PREFIX
22
assert(argv0_path);
@@ -34,8 +34,7 @@ const char *system_path(const char *path)
34
#endif
35
36
strbuf_addf(&d, "%s/%s", prefix, path);
37
- path = strbuf_detach(&d, NULL);
38
- return path;
37
+ return strbuf_detach(&d, NULL);
38
}
39
40
const char *git_extract_argv0_path(const char *argv0)
exec_cmd.h
+1
-1
@@ -9,6 +9,6 @@ extern const char **prepare_git_cmd(const char **argv);
9
extern int execv_git_cmd(const char **argv); /* NULL terminated */
10
LAST_ARG_MUST_BE_NULL
11
extern int execl_git_cmd(const char *cmd, ...);
12
-extern const char *system_path(const char *path);
12
+extern char *system_path(const char *path);
13
14
#endif /* GIT_EXEC_CMD_H */