undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

real_path: remove unsafe API

Browse Source 
Returning a shared buffer invites very subtle bugs due to reentrancy or
multi-threading, as demonstrated by the previous patch.

There was an unfinished effort to abolish this [1].

Let's finally rid of `real_path()`, using `strbuf_realpath()` instead.

This patch uses a local `strbuf` for most places where `real_path()` was
previously called.

However, two places return the value of `real_path()` to the caller. For
them, a `static` local `strbuf` was added, effectively pushing the
problem one level higher:
    read_gitfile_gently()
    get_superproject_working_tree()

[1] https://lore.kernel.org/git/1480964316-99305-1-git-send-email-bmwill@google.com/

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Alexandr Miloslavskiy 2020-03-10 13:11:22 +00:00 committed by Junio C Hamano
parent 0915a5b4cd
commit 3d7747e318
13 changed files with 59 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
abspath.c
View File

@ -206,12 +206,6 @@ error_out:
* Resolve `path` into an absolute, cleaned-up path. The return value
* comes from a shared buffer.
*/
const char *real_path(const char *path)
{
static struct strbuf realpath = STRBUF_INIT;
return strbuf_realpath(&realpath, path, 1);
}
const char *real_path_if_valid(const char *path)
{
static struct strbuf realpath = STRBUF_INIT;
@ -233,7 +227,7 @@ char *real_pathdup(const char *path, int die_on_error)
/*
* Use this to get an absolute path from a relative one. If you want
* to resolve links, you should use real_path.
* to resolve links, you should use strbuf_realpath.
*/
const char *absolute_path(const char *path)
{

6
builtin/clone.c
View File

@ -420,6 +420,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
struct dir_iterator *iter;
int iter_status;
unsigned int flags;
struct strbuf realpath = STRBUF_INIT;
mkdir_if_missing(dest->buf, 0777);
@ -454,7 +455,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
if (unlink(dest->buf) && errno != ENOENT)
die_errno(_("failed to unlink '%s'"), dest->buf);
if (!option_no_hardlinks) {
if (!link(real_path(src->buf), dest->buf))
strbuf_realpath(&realpath, src->buf, 1);
if (!link(realpath.buf, dest->buf))
continue;
if (option_local > 0)
die_errno(_("failed to create link '%s'"), dest->buf);
@ -468,6 +470,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
strbuf_setlen(src, src_len);
die(_("failed to iterate over '%s'"), src->buf);
}
strbuf_release(&realpath);
}
static void clone_local(const char *src_repo, const char *dest_repo)

5
builtin/commit-graph.c
View File

@ -39,14 +39,17 @@ static struct object_directory *find_odb(struct repository *r,
{
struct object_directory *odb;
char *obj_dir_real = real_pathdup(obj_dir, 1);
struct strbuf odb_path_real = STRBUF_INIT;
prepare_alt_odb(r);
for (odb = r->objects->odb; odb; odb = odb->next) {
if (!strcmp(obj_dir_real, real_path(odb->path)))
strbuf_realpath(&odb_path_real, odb->path, 1);
if (!strcmp(obj_dir_real, odb_path_real.buf))
break;
}
free(obj_dir_real);
strbuf_release(&odb_path_real);
if (!odb)
die(_("could not find object directory matching %s"), obj_dir);

5
builtin/rev-parse.c
View File

@ -857,7 +857,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
if (!gitdir && !prefix)
gitdir = ".git";
if (gitdir) {
puts(real_path(gitdir));
struct strbuf realpath = STRBUF_INIT;
strbuf_realpath(&realpath, gitdir, 1);
puts(realpath.buf);
strbuf_release(&realpath);
continue;
}
}

9
builtin/worktree.c
View File

@ -258,7 +258,7 @@ static int add_worktree(const char *path, const char *refname,
const struct add_opts *opts)
{
struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT;
const char *name;
struct child_process cp = CHILD_PROCESS_INIT;
struct argv_array child_env = ARGV_ARRAY_INIT;
@ -330,9 +330,11 @@ static int add_worktree(const char *path, const char *refname,
strbuf_reset(&sb);
strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
write_file(sb.buf, "%s", real_path(sb_git.buf));
strbuf_realpath(&realpath, sb_git.buf, 1);
write_file(sb.buf, "%s", realpath.buf);
strbuf_realpath(&realpath, get_git_common_dir(), 1);
write_file(sb_git.buf, "gitdir: %s/worktrees/%s",
real_path(get_git_common_dir()), name);
realpath.buf, name);
/*
* This is to keep resolve_ref() happy. We need a valid HEAD
* or is_git_directory() will reject the directory. Any value which
@ -418,6 +420,7 @@ done:
strbuf_release(&sb_repo);
strbuf_release(&sb_git);
strbuf_release(&sb_name);
strbuf_release(&realpath);
return ret;
}

1
cache.h
View File

@ -1314,7 +1314,6 @@ static inline int is_absolute_path(const char *path)
int is_directory(const char *);
char *strbuf_realpath(struct strbuf *resolved, const char *path,
int die_on_error);
const char *real_path(const char *path);
const char *real_path_if_valid(const char *path);
char *real_pathdup(const char *path, int die_on_error);
const char *absolute_path(const char *path);

11
editor.c
View File

@ -54,7 +54,8 @@ static int launch_specified_editor(const char *editor, const char *path,
return error("Terminal is dumb, but EDITOR unset");
if (strcmp(editor, ":")) {
const char *args[] = { editor, real_path(path), NULL };
struct strbuf realpath = STRBUF_INIT;
const char *args[] = { editor, NULL, NULL };
struct child_process p = CHILD_PROCESS_INIT;
int ret, sig;
int print_waiting_for_editor = advice_waiting_for_editor && isatty(2);
@ -75,16 +76,22 @@ static int launch_specified_editor(const char *editor, const char *path,
fflush(stderr);
}
strbuf_realpath(&realpath, path, 1);
args[1] = realpath.buf;
p.argv = args;
p.env = env;
p.use_shell = 1;
p.trace2_child_class = "editor";
if (start_command(&p) < 0)
if (start_command(&p) < 0) {
strbuf_release(&realpath);
return error("unable to start editor '%s'", editor);
}
sigchain_push(SIGINT, SIG_IGN);
sigchain_push(SIGQUIT, SIG_IGN);
ret = finish_command(&p);
strbuf_release(&realpath);
sig = ret - 128;
sigchain_pop(SIGINT);
sigchain_pop(SIGQUIT);

7
environment.c
View File

@ -254,8 +254,11 @@ static int git_work_tree_initialized;
*/
void set_git_work_tree(const char *new_work_tree)
{
struct strbuf realpath = STRBUF_INIT;
if (git_work_tree_initialized) {
new_work_tree = real_path(new_work_tree);
strbuf_realpath(&realpath, new_work_tree, 1);
new_work_tree = realpath.buf;
if (strcmp(new_work_tree, the_repository->worktree))
die("internal error: work tree has already been set\n"
"Current worktree: %s\nNew worktree: %s",
@ -264,6 +267,8 @@ void set_git_work_tree(const char *new_work_tree)
}
git_work_tree_initialized = 1;
repo_set_worktree(the_repository, new_work_tree);
strbuf_release(&realpath);
}
const char *get_git_work_tree(void)

2
path.c
View File

@ -723,7 +723,7 @@ static struct passwd *getpw_str(const char *username, size_t len)
* then it is a newly allocated string. Returns NULL on getpw failure or
* if path is NULL.
*
* If real_home is true, real_path($HOME) is used in the expansion.
* If real_home is true, strbuf_realpath($HOME) is used in the expansion.
*/
char *expand_user_path(const char *path, int real_home)
{

15
setup.c
View File

@ -32,6 +32,7 @@ static int abspath_part_inside_repo(char *path)
char *path0;
int off;
const char *work_tree = get_git_work_tree();
struct strbuf realpath = STRBUF_INIT;
if (!work_tree)
return -1;
@ -60,8 +61,10 @@ static int abspath_part_inside_repo(char *path)
path++;
if (*path == '/') {
*path = '\0';
if (fspathcmp(real_path(path0), work_tree) == 0) {
strbuf_realpath(&realpath, path0, 1);
if (fspathcmp(realpath.buf, work_tree) == 0) {
memmove(path0, path + 1, len - (path - path0));
strbuf_release(&realpath);
return 0;
}
*path = '/';
@ -69,11 +72,14 @@ static int abspath_part_inside_repo(char *path)
}
/* check whole path */
if (fspathcmp(real_path(path0), work_tree) == 0) {
strbuf_realpath(&realpath, path0, 1);
if (fspathcmp(realpath.buf, work_tree) == 0) {
*path0 = '\0';
strbuf_release(&realpath);
return 0;
}
strbuf_release(&realpath);
return -1;
}
@ -619,6 +625,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
struct stat st;
int fd;
ssize_t len;
static struct strbuf realpath = STRBUF_INIT;
if (stat(path, &st)) {
/* NEEDSWORK: discern between ENOENT vs other errors */
@ -669,7 +676,9 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
error_code = READ_GITFILE_ERR_NOT_A_REPO;
goto cleanup_return;
}
path = real_path(dir);
strbuf_realpath(&realpath, dir, 1);
path = realpath.buf;
cleanup_return:
if (return_error_code)

4
submodule.c
View File

@ -2170,6 +2170,7 @@ void absorb_git_dir_into_superproject(const char *path,
const char *get_superproject_working_tree(void)
{
static struct strbuf realpath = STRBUF_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf sb = STRBUF_INIT;
const char *one_up = real_path_if_valid("../");
@ -2231,7 +2232,8 @@ const char *get_superproject_working_tree(void)
super_wt = xstrdup(cwd);
super_wt[cwd_len - super_sub_len] = '\0';
ret = real_path(super_wt);
strbuf_realpath(&realpath, super_wt, 1);
ret = realpath.buf;
free(super_wt);
}
strbuf_release(&sb);

5
t/helper/test-path-utils.c
View File

@ -290,11 +290,14 @@ int cmd__path_utils(int argc, const char **argv)
}
if (argc >= 2 && !strcmp(argv[1], "real_path")) {
struct strbuf realpath = STRBUF_INIT;
while (argc > 2) {
puts(real_path(argv[2]));
strbuf_realpath(&realpath, argv[2], 1);
puts(realpath.buf);
argc--;
argv++;
}
strbuf_release(&realpath);
return 0;
}

5
worktree.c
View File

@ -285,6 +285,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,
unsigned flags)
{
struct strbuf wt_path = STRBUF_INIT;
struct strbuf realpath = STRBUF_INIT;
char *path = NULL;
int err, ret = -1;
@ -336,7 +337,8 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,
goto done;
}
ret = fspathcmp(path, real_path(git_common_path("worktrees/%s", wt->id)));
strbuf_realpath(&realpath, git_common_path("worktrees/%s", wt->id), 1);
ret = fspathcmp(path, realpath.buf);
if (ret)
strbuf_addf_gently(errmsg, _("'%s' does not point back to '%s'"),
@ -344,6 +346,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,
done:
free(path);
strbuf_release(&wt_path);
strbuf_release(&realpath);
return ret;
}