Am 21.06.21 um 22:49 schrieb Elijah Newren:
> On Sun, Jun 20, 2021 at 8:14 AM <andrzej@xxxxxxxxx> wrote:
>> It looks like this leak has existed since realpath was first added to
>> set_git_work_tree() in:
>> 3d7747e318 (real_path: remove unsafe API, 2020-03-10)
>
> Looking at that commit, it appears to have introduced other problems.
> For example, the documentation for read_gitfile_gently() claims it
> returns a value from a shared buffer, but that commit got rid of the
> shared buffer so the documentation is no longer accurate. The thing
> that is returned is either the path that was passed in, or some newly
> allocated path that differs, in which case the caller would be
> responsible to free() it, but it looks like the callers aren't doing
> so.
That comment is still correct. The returned pointer references a shared
static buffer declared in read_gitfile_gently(). The control flow is a
bit hard to follow; path points to the static buffer if and only if
error_code is zero. Using a dedicated variable for the result would
make that clearer, I think:
diff --git a/setup.c b/setup.c
index ead2f80cd8..75b0a4bea6 100644
--- a/setup.c
+++ b/setup.c
@@ -720,86 +720,87 @@ void read_gitfile_error_die(int error_code, const char *path, const char *dir)
/*
* Try to read the location of the git directory from the .git file,
* return path to git directory if found. The return value comes from
* a shared buffer.
*
* On failure, if return_error_code is not NULL, return_error_code
* will be set to an error code and NULL will be returned. If
* return_error_code is NULL the function will die instead (for most
* cases).
*/
const char *read_gitfile_gently(const char *path, int *return_error_code)
{
const int max_file_size = 1 << 20; /* 1MB */
int error_code = 0;
char *buf = NULL;
char *dir = NULL;
const char *slash;
struct stat st;
int fd;
ssize_t len;
static struct strbuf realpath = STRBUF_INIT;
+ const char *result = NULL;
if (stat(path, &st)) {
/* NEEDSWORK: discern between ENOENT vs other errors */
error_code = READ_GITFILE_ERR_STAT_FAILED;
goto cleanup_return;
}
if (!S_ISREG(st.st_mode)) {
error_code = READ_GITFILE_ERR_NOT_A_FILE;
goto cleanup_return;
}
if (st.st_size > max_file_size) {
error_code = READ_GITFILE_ERR_TOO_LARGE;
goto cleanup_return;
}
fd = open(path, O_RDONLY);
if (fd < 0) {
error_code = READ_GITFILE_ERR_OPEN_FAILED;
goto cleanup_return;
}
buf = xmallocz(st.st_size);
len = read_in_full(fd, buf, st.st_size);
close(fd);
if (len != st.st_size) {
error_code = READ_GITFILE_ERR_READ_FAILED;
goto cleanup_return;
}
if (!starts_with(buf, "gitdir: ")) {
error_code = READ_GITFILE_ERR_INVALID_FORMAT;
goto cleanup_return;
}
while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
len--;
if (len < 9) {
error_code = READ_GITFILE_ERR_NO_PATH;
goto cleanup_return;
}
buf[len] = '\0';
dir = buf + 8;
if (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {
size_t pathlen = slash+1 - path;
dir = xstrfmt("%.*s%.*s", (int)pathlen, path,
(int)(len - 8), buf + 8);
free(buf);
buf = dir;
}
if (!is_git_directory(dir)) {
error_code = READ_GITFILE_ERR_NOT_A_REPO;
goto cleanup_return;
}
strbuf_realpath(&realpath, dir, 1);
- path = realpath.buf;
+ result = realpath.buf;
cleanup_return:
if (return_error_code)
*return_error_code = error_code;
else if (error_code)
read_gitfile_error_die(error_code, path, dir);
free(buf);
- return error_code ? NULL : path;
+ return result;
}
static const char *setup_explicit_git_dir(const char *gitdirenv,
