* [PATCH 3/3] rename git_path() to git_path_unsafe()
From: Jonathan Nieder @ 2011-11-16 8:07 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Junio C Hamano, Git List, Nguyễn Thái Ngọc Duy
In-Reply-To: <20111116075955.GB13706@elie.hsd1.il.comcast.net>
git_path() stores its result to one of a rotating collection of four
static buffers. If more than 4 git_path() results are in play, the
result can be a little unpleasant, as each call clobbers the return
value from previous calls.
Therefore callers should be careful not to assign the return value
from git_path() to a long-lived variable. Rename the function to
git_path_unsafe() as a reminder.
Mechanics: This patch only makes three kinds of changes:
1) changing git_path(foo) to git_path_unsafe(foo)
2) changing xstrdup(git_path(foo)) to git_pathdup(foo)
3) rewrapping lines that were made longer by (1)
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/technical/api-string-list.txt | 5 +-
attr.c | 2 +-
bisect.c | 8 ++--
branch.c | 12 +++---
builtin/add.c | 2 +-
builtin/commit.c | 57 ++++++++++++++-------------
builtin/config.c | 4 +-
builtin/fetch-pack.c | 4 +-
builtin/fetch.c | 5 +-
builtin/fsck.c | 2 +-
builtin/init-db.c | 12 +++---
builtin/merge.c | 32 +++++++-------
builtin/notes.c | 2 +-
builtin/remote.c | 6 +-
builtin/reset.c | 2 +-
builtin/revert.c | 18 ++++----
cache.h | 3 +-
contrib/examples/builtin-fetch--tool.c | 4 +-
dir.c | 2 +-
fast-import.c | 2 +-
http-backend.c | 2 +-
notes-merge.c | 22 ++++++-----
pack-refs.c | 6 +-
path.c | 2 +-
refs.c | 51 +++++++++++++-----------
remote.c | 4 +-
rerere.c | 12 +++---
run-command.c | 4 +-
sequencer.c | 6 +-
server-info.c | 2 +-
sha1_file.c | 4 +-
shallow.c | 2 +-
transport.c | 4 +-
unpack-trees.c | 2 +-
34 files changed, 160 insertions(+), 147 deletions(-)
diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt
index ce24eb96..446a51ab 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -13,8 +13,9 @@ The caller:
. Initializes the members. You might want to set the flag `strdup_strings`
if the strings should be strdup()ed. For example, this is necessary
- when you add something like git_path("..."), since that function returns
- a static buffer that will change with the next call to git_path().
+ when you add something like git_path_unsafe("..."), since that function
+ returns a static buffer that will change with the next call to
+ git_path_unsafe().
+
If you need something advanced, you can manually malloc() the `items`
member (you need this if you add things later) and you should set the
diff --git a/attr.c b/attr.c
index 76b079f0..afdd6d24 100644
--- a/attr.c
+++ b/attr.c
@@ -529,7 +529,7 @@ static void bootstrap_attr_stack(void)
debug_push(elem);
}
- elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1);
+ elem = read_attr_from_file(git_path_unsafe(INFOATTRIBUTES_FILE), 1);
if (!elem)
elem = xcalloc(1, sizeof(*elem));
elem->origin = NULL;
diff --git a/bisect.c b/bisect.c
index 6e186e29..315d22e6 100644
--- a/bisect.c
+++ b/bisect.c
@@ -422,7 +422,7 @@ static int read_bisect_refs(void)
static void read_bisect_paths(struct argv_array *array)
{
struct strbuf str = STRBUF_INIT;
- const char *filename = git_path("BISECT_NAMES");
+ const char *filename = git_path_unsafe("BISECT_NAMES");
FILE *fp = fopen(filename, "r");
if (!fp)
@@ -643,7 +643,7 @@ static void exit_if_skipped_commits(struct commit_list *tried,
static int is_expected_rev(const unsigned char *sha1)
{
- const char *filename = git_path("BISECT_EXPECTED_REV");
+ const char *filename = git_path_unsafe("BISECT_EXPECTED_REV");
struct stat st;
struct strbuf str = STRBUF_INIT;
FILE *fp;
@@ -668,7 +668,7 @@ static int is_expected_rev(const unsigned char *sha1)
static void mark_expected_rev(char *bisect_rev_hex)
{
int len = strlen(bisect_rev_hex);
- const char *filename = git_path("BISECT_EXPECTED_REV");
+ const char *filename = git_path_unsafe("BISECT_EXPECTED_REV");
int fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
if (fd < 0)
@@ -833,7 +833,7 @@ static int check_ancestors(const char *prefix)
*/
static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
{
- const char *filename = git_path("BISECT_ANCESTORS_OK");
+ const char *filename = git_path_unsafe("BISECT_ANCESTORS_OK");
struct stat st;
int fd;
diff --git a/branch.c b/branch.c
index d8098762..5a3faa10 100644
--- a/branch.c
+++ b/branch.c
@@ -240,11 +240,11 @@ void create_branch(const char *head,
void remove_branch_state(void)
{
- unlink(git_path("CHERRY_PICK_HEAD"));
- unlink(git_path("MERGE_HEAD"));
- unlink(git_path("MERGE_RR"));
- unlink(git_path("MERGE_MSG"));
- unlink(git_path("MERGE_MODE"));
- unlink(git_path("SQUASH_MSG"));
+ unlink(git_path_unsafe("CHERRY_PICK_HEAD"));
+ unlink(git_path_unsafe("MERGE_HEAD"));
+ unlink(git_path_unsafe("MERGE_RR"));
+ unlink(git_path_unsafe("MERGE_MSG"));
+ unlink(git_path_unsafe("MERGE_MODE"));
+ unlink(git_path_unsafe("SQUASH_MSG"));
remove_sequencer_state(0);
}
diff --git a/builtin/add.c b/builtin/add.c
index c59b0c98..0aabb61d 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -259,7 +259,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch)
static int edit_patch(int argc, const char **argv, const char *prefix)
{
- char *file = xstrdup(git_path("ADD_EDIT.patch"));
+ char *file = git_pathdup("ADD_EDIT.patch");
const char *apply_argv[] = { "apply", "--recount", "--cached",
NULL, NULL };
struct child_process child;
diff --git a/builtin/commit.c b/builtin/commit.c
index c46f2d18..e9aa5e75 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -178,9 +178,9 @@ static struct option builtin_commit_options[] = {
static void determine_whence(struct wt_status *s)
{
- if (file_exists(git_path("MERGE_HEAD")))
+ if (file_exists(git_path_unsafe("MERGE_HEAD")))
whence = FROM_MERGE;
- else if (file_exists(git_path("CHERRY_PICK_HEAD")))
+ else if (file_exists(git_path_unsafe("CHERRY_PICK_HEAD")))
whence = FROM_CHERRY_PICK;
else
whence = FROM_COMMIT;
@@ -465,8 +465,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
die(_("unable to write new_index file"));
fd = hold_lock_file_for_update(&false_lock,
- git_path("next-index-%"PRIuMAX,
- (uintmax_t) getpid()),
+ git_path_unsafe("next-index-%"PRIuMAX,
+ (uintmax_t) getpid()),
LOCK_DIE_ON_ERROR);
create_base_index(current_head);
@@ -691,12 +691,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
format_commit_message(commit, "fixup! %s\n\n",
&sb, &ctx);
hook_arg1 = "message";
- } else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
- if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
+ } else if (!stat(git_path_unsafe("MERGE_MSG"), &statbuf)) {
+ if (strbuf_read_file(&sb, git_path_unsafe("MERGE_MSG"), 0) < 0)
die_errno(_("could not read MERGE_MSG"));
hook_arg1 = "merge";
- } else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
- if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
+ } else if (!stat(git_path_unsafe("SQUASH_MSG"), &statbuf)) {
+ if (strbuf_read_file(&sb, git_path_unsafe("SQUASH_MSG"), 0) < 0)
die_errno(_("could not read SQUASH_MSG"));
hook_arg1 = "squash";
} else if (template_file) {
@@ -727,9 +727,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
hook_arg2 = "";
}
- s->fp = fopen(git_path(commit_editmsg), "w");
+ s->fp = fopen(git_path_unsafe(commit_editmsg), "w");
if (s->fp == NULL)
- die_errno(_("could not open '%s'"), git_path(commit_editmsg));
+ die_errno(_("could not open '%s'"),
+ git_path_unsafe(commit_editmsg));
if (clean_message_contents)
stripspace(&sb, 0);
@@ -773,7 +774,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
"and try again.\n"
""),
whence_s(),
- git_path(whence == FROM_MERGE
+ git_path_unsafe(whence == FROM_MERGE
? "MERGE_HEAD"
: "CHERRY_PICK_HEAD"));
@@ -870,8 +871,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
return 0;
}
- if (run_hook(index_file, "prepare-commit-msg",
- git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+ if (run_hook(index_file,
+ "prepare-commit-msg", git_path_unsafe(commit_editmsg),
+ hook_arg1, hook_arg2, NULL))
return 0;
if (use_editor) {
@@ -879,7 +881,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
const char *env[2] = { NULL };
env[0] = index;
snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
- if (launch_editor(git_path(commit_editmsg), NULL, env)) {
+ if (launch_editor(git_path_unsafe(commit_editmsg), NULL, env)) {
fprintf(stderr,
_("Please supply the message using either -m or -F option.\n"));
exit(1);
@@ -887,7 +889,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
}
if (!no_verify &&
- run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
+ run_hook(index_file,
+ "commit-msg", git_path_unsafe(commit_editmsg), NULL)) {
return 0;
}
@@ -1347,10 +1350,10 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
int code;
size_t n;
- if (access(git_path(post_rewrite_hook), X_OK) < 0)
+ if (access(git_path_unsafe(post_rewrite_hook), X_OK) < 0)
return 0;
- argv[0] = git_path(post_rewrite_hook);
+ argv[0] = git_path_unsafe(post_rewrite_hook);
argv[1] = "amend";
argv[2] = NULL;
@@ -1431,10 +1434,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!reflog_msg)
reflog_msg = "commit (merge)";
pptr = &commit_list_insert(current_head, pptr)->next;
- fp = fopen(git_path("MERGE_HEAD"), "r");
+ fp = fopen(git_path_unsafe("MERGE_HEAD"), "r");
if (fp == NULL)
die_errno(_("could not open '%s' for reading"),
- git_path("MERGE_HEAD"));
+ git_path_unsafe("MERGE_HEAD"));
while (strbuf_getline(&m, fp, '\n') != EOF) {
unsigned char sha1[20];
if (get_sha1_hex(m.buf, sha1) < 0)
@@ -1444,8 +1447,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
fclose(fp);
strbuf_release(&m);
- if (!stat(git_path("MERGE_MODE"), &statbuf)) {
- if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) < 0)
+ if (!stat(git_path_unsafe("MERGE_MODE"), &statbuf)) {
+ if (strbuf_read_file(&sb, git_path_unsafe("MERGE_MODE"), 0) < 0)
die_errno(_("could not read MERGE_MODE"));
if (!strcmp(sb.buf, "no-ff"))
allow_fast_forward = 0;
@@ -1462,7 +1465,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
/* Finally, get the commit message */
strbuf_reset(&sb);
- if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
+ if (strbuf_read_file(&sb, git_path_unsafe(commit_editmsg), 0) < 0) {
int saved_errno = errno;
rollback_index_files();
die(_("could not read commit message: %s"), strerror(saved_errno));
@@ -1513,11 +1516,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
die(_("cannot update HEAD ref"));
}
- unlink(git_path("CHERRY_PICK_HEAD"));
- unlink(git_path("MERGE_HEAD"));
- unlink(git_path("MERGE_MSG"));
- unlink(git_path("MERGE_MODE"));
- unlink(git_path("SQUASH_MSG"));
+ unlink(git_path_unsafe("CHERRY_PICK_HEAD"));
+ unlink(git_path_unsafe("MERGE_HEAD"));
+ unlink(git_path_unsafe("MERGE_MSG"));
+ unlink(git_path_unsafe("MERGE_MODE"));
+ unlink(git_path_unsafe("SQUASH_MSG"));
if (commit_index_files())
die (_("Repository has been updated, but unable to write\n"
diff --git a/builtin/config.c b/builtin/config.c
index 0315ad76..407d7ca8 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -434,8 +434,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
die("not in a git directory");
git_config(git_default_config, NULL);
launch_editor(config_exclusive_filename ?
- config_exclusive_filename : git_path("config"),
- NULL, NULL);
+ config_exclusive_filename :
+ git_path_unsafe("config"), NULL, NULL);
}
else if (actions == ACTION_SET) {
int ret;
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index c6bc8eb0..f8d0954c 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1026,7 +1026,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
if (&args != my_args)
memcpy(&args, my_args, sizeof(args));
if (args.depth > 0) {
- if (stat(git_path("shallow"), &st))
+ if (stat(git_path_unsafe("shallow"), &st))
st.st_mtime = 0;
}
@@ -1041,7 +1041,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
if (args.depth > 0) {
struct cache_time mtime;
struct strbuf sb = STRBUF_INIT;
- char *shallow = git_path("shallow");
+ char *shallow = git_path_unsafe("shallow");
int fd;
mtime.sec = st.st_mtime;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 91731b90..ccbe63c7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -367,7 +367,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
char note[1024];
const char *what, *kind;
struct ref *rm;
- char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+ char *url;
+ char *filename = dry_run ? "/dev/null" : git_path_unsafe("FETCH_HEAD");
fp = fopen(filename, "a");
if (!fp)
@@ -647,7 +648,7 @@ static void check_not_current_branch(struct ref *ref_map)
static int truncate_fetch_head(void)
{
- char *filename = git_path("FETCH_HEAD");
+ char *filename = git_path_unsafe("FETCH_HEAD");
FILE *fp = fopen(filename, "w");
if (!fp)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index df1a88b5..f8429f55 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -215,7 +215,7 @@ static void check_unreachable_object(struct object *obj)
printf("dangling %s %s\n", typename(obj->type),
sha1_to_hex(obj->sha1));
if (write_lost_and_found) {
- char *filename = git_path("lost-found/%s/%s",
+ char *filename = git_path_unsafe("lost-found/%s/%s",
obj->type == OBJ_COMMIT ? "commit" : "other",
sha1_to_hex(obj->sha1));
FILE *f;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index d07554c8..7ae89f85 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -198,9 +198,9 @@ static int create_default_files(const char *template_path)
/*
* Create .git/refs/{heads,tags}
*/
- safe_create_dir(git_path("refs"), 1);
- safe_create_dir(git_path("refs/heads"), 1);
- safe_create_dir(git_path("refs/tags"), 1);
+ safe_create_dir(git_path_unsafe("refs"), 1);
+ safe_create_dir(git_path_unsafe("refs/heads"), 1);
+ safe_create_dir(git_path_unsafe("refs/tags"), 1);
/* Just look for `init.templatedir` */
git_config(git_init_db_config, NULL);
@@ -224,9 +224,9 @@ static int create_default_files(const char *template_path)
*/
if (shared_repository) {
adjust_shared_perm(get_git_dir());
- adjust_shared_perm(git_path("refs"));
- adjust_shared_perm(git_path("refs/heads"));
- adjust_shared_perm(git_path("refs/tags"));
+ adjust_shared_perm(git_path_unsafe("refs"));
+ adjust_shared_perm(git_path_unsafe("refs/heads"));
+ adjust_shared_perm(git_path_unsafe("refs/tags"));
}
/*
diff --git a/builtin/merge.c b/builtin/merge.c
index 2870a6af..3e75c30b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -213,9 +213,9 @@ static struct option builtin_merge_options[] = {
/* Cleans up metadata that is uninteresting after a succeeded merge. */
static void drop_save(void)
{
- unlink(git_path("MERGE_HEAD"));
- unlink(git_path("MERGE_MSG"));
- unlink(git_path("MERGE_MODE"));
+ unlink(git_path_unsafe("MERGE_HEAD"));
+ unlink(git_path_unsafe("MERGE_MSG"));
+ unlink(git_path_unsafe("MERGE_MODE"));
}
static int save_state(unsigned char *stash)
@@ -321,7 +321,7 @@ static void squash_message(struct commit *commit)
struct pretty_print_context ctx = {0};
printf(_("Squash commit -- not updating HEAD\n"));
- filename = git_path("SQUASH_MSG");
+ filename = git_path_unsafe("SQUASH_MSG");
fd = open(filename, O_WRONLY | O_CREAT, 0666);
if (fd < 0)
die_errno(_("Could not write to '%s'"), filename);
@@ -493,13 +493,13 @@ static void merge_name(const char *remote, struct strbuf *msg)
}
if (!strcmp(remote, "FETCH_HEAD") &&
- !access(git_path("FETCH_HEAD"), R_OK)) {
+ !access(git_path_unsafe("FETCH_HEAD"), R_OK)) {
const char *filename;
FILE *fp;
struct strbuf line = STRBUF_INIT;
char *ptr;
- filename = git_path("FETCH_HEAD");
+ filename = git_path_unsafe("FETCH_HEAD");
fp = fopen(filename, "r");
if (!fp)
die_errno(_("could not open '%s' for reading"),
@@ -851,7 +851,7 @@ static void add_strategies(const char *string, unsigned attr)
static void write_merge_msg(struct strbuf *msg)
{
- const char *filename = git_path("MERGE_MSG");
+ const char *filename = git_path_unsafe("MERGE_MSG");
int fd = open(filename, O_WRONLY | O_CREAT, 0666);
if (fd < 0)
die_errno(_("Could not open '%s' for writing"),
@@ -863,7 +863,7 @@ static void write_merge_msg(struct strbuf *msg)
static void read_merge_msg(struct strbuf *msg)
{
- const char *filename = git_path("MERGE_MSG");
+ const char *filename = git_path_unsafe("MERGE_MSG");
strbuf_reset(msg);
if (strbuf_read_file(msg, filename, 0) < 0)
die_errno(_("Could not read from '%s'"), filename);
@@ -887,9 +887,9 @@ static void prepare_to_commit(void)
strbuf_addch(&msg, '\n');
write_merge_msg(&msg);
run_hook(get_index_file(), "prepare-commit-msg",
- git_path("MERGE_MSG"), "merge", NULL, NULL);
+ git_path_unsafe("MERGE_MSG"), "merge", NULL, NULL);
if (option_edit) {
- if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
+ if (launch_editor(git_path_unsafe("MERGE_MSG"), NULL, NULL))
abort_commit(NULL);
}
read_merge_msg(&msg);
@@ -958,7 +958,7 @@ static int suggest_conflicts(int renormalizing)
FILE *fp;
int pos;
- filename = git_path("MERGE_MSG");
+ filename = git_path_unsafe("MERGE_MSG");
fp = fopen(filename, "a");
if (!fp)
die_errno(_("Could not open '%s' for writing"), filename);
@@ -1061,7 +1061,7 @@ static void write_merge_state(void)
for (j = remoteheads; j; j = j->next)
strbuf_addf(&buf, "%s\n",
sha1_to_hex(j->item->object.sha1));
- filename = git_path("MERGE_HEAD");
+ filename = git_path_unsafe("MERGE_HEAD");
fd = open(filename, O_WRONLY | O_CREAT, 0666);
if (fd < 0)
die_errno(_("Could not open '%s' for writing"), filename);
@@ -1071,7 +1071,7 @@ static void write_merge_state(void)
strbuf_addch(&merge_msg, '\n');
write_merge_msg(&merge_msg);
- filename = git_path("MERGE_MODE");
+ filename = git_path_unsafe("MERGE_MODE");
fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666);
if (fd < 0)
die_errno(_("Could not open '%s' for writing"), filename);
@@ -1126,7 +1126,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
int nargc = 2;
const char *nargv[] = {"reset", "--merge", NULL};
- if (!file_exists(git_path("MERGE_HEAD")))
+ if (!file_exists(git_path_unsafe("MERGE_HEAD")))
die(_("There is no merge to abort (MERGE_HEAD missing)."));
/* Invoke 'git reset --merge' */
@@ -1136,7 +1136,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (read_cache_unmerged())
die_resolve_conflict("merge");
- if (file_exists(git_path("MERGE_HEAD"))) {
+ if (file_exists(git_path_unsafe("MERGE_HEAD"))) {
/*
* There is no unmerged entry, don't advise 'git
* add/rm <file>', just 'git commit'.
@@ -1147,7 +1147,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
else
die(_("You have not concluded your merge (MERGE_HEAD exists)."));
}
- if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
+ if (file_exists(git_path_unsafe("CHERRY_PICK_HEAD"))) {
if (advice_resolve_conflict)
die(_("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n"
"Please, commit your changes before you can merge."));
diff --git a/builtin/notes.c b/builtin/notes.c
index f8e437db..c037afe6 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -944,7 +944,7 @@ static int merge(int argc, const char **argv, const char *prefix)
printf("Automatic notes merge failed. Fix conflicts in %s and "
"commit the result with 'git notes merge --commit', or "
"abort the merge with 'git notes merge --abort'.\n",
- git_path(NOTES_MERGE_WORKTREE));
+ git_path_unsafe(NOTES_MERGE_WORKTREE));
}
free_notes(t);
diff --git a/builtin/remote.c b/builtin/remote.c
index c8106438..c7032125 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -546,7 +546,7 @@ static int add_branch_for_removal(const char *refname,
/* make sure that symrefs are deleted */
if (flags & REF_ISSYMREF)
- return unlink(git_path("%s", refname));
+ return unlink(git_path_unsafe("%s", refname));
item = string_list_append(branches->branches, refname);
item->util = xmalloc(20);
@@ -608,9 +608,9 @@ static int migrate_file(struct remote *remote)
return error("Could not append '%s' to '%s'",
remote->fetch_refspec[i], buf.buf);
if (remote->origin == REMOTE_REMOTES)
- path = git_path("remotes/%s", remote->name);
+ path = git_path_unsafe("remotes/%s", remote->name);
else if (remote->origin == REMOTE_BRANCHES)
- path = git_path("branches/%s", remote->name);
+ path = git_path_unsafe("branches/%s", remote->name);
if (path)
unlink_or_warn(path);
return 0;
diff --git a/builtin/reset.c b/builtin/reset.c
index 811e8e25..18bacdac 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -35,7 +35,7 @@ static const char *reset_type_names[] = {
static inline int is_merge(void)
{
- return !access(git_path("MERGE_HEAD"), F_OK);
+ return !access(git_path_unsafe("MERGE_HEAD"), F_OK);
}
static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)
diff --git a/builtin/revert.c b/builtin/revert.c
index 985f95b0..09a062c6 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -294,7 +294,7 @@ static void write_cherry_pick_head(struct commit *commit)
strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
- filename = git_path("CHERRY_PICK_HEAD");
+ filename = git_path_unsafe("CHERRY_PICK_HEAD");
fd = open(filename, O_WRONLY | O_CREAT, 0666);
if (fd < 0)
die_errno(_("Could not open '%s' for writing"), filename);
@@ -314,7 +314,7 @@ static void print_advice(int show_hint)
* (typically rebase --interactive) wants to take care
* of the commit itself so remove CHERRY_PICK_HEAD
*/
- unlink(git_path("CHERRY_PICK_HEAD"));
+ unlink(git_path_unsafe("CHERRY_PICK_HEAD"));
return;
}
@@ -762,7 +762,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
static void read_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
{
- const char *todo_file = git_path(SEQ_TODO_FILE);
+ const char *todo_file = git_path_unsafe(SEQ_TODO_FILE);
struct strbuf buf = STRBUF_INIT;
int fd, res;
@@ -817,7 +817,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
static void read_populate_opts(struct replay_opts **opts_ptr)
{
- const char *opts_file = git_path(SEQ_OPTS_FILE);
+ const char *opts_file = git_path_unsafe(SEQ_OPTS_FILE);
if (!file_exists(opts_file))
return;
@@ -841,7 +841,7 @@ static void walk_revs_populate_todo(struct commit_list **todo_list,
static int create_seq_dir(void)
{
- const char *seq_dir = git_path(SEQ_DIR);
+ const char *seq_dir = git_path_unsafe(SEQ_DIR);
if (file_exists(seq_dir))
return error(_("%s already exists."), seq_dir);
@@ -852,7 +852,7 @@ static int create_seq_dir(void)
static void save_head(const char *head)
{
- const char *head_file = git_path(SEQ_HEAD_FILE);
+ const char *head_file = git_path_unsafe(SEQ_HEAD_FILE);
static struct lock_file head_lock;
struct strbuf buf = STRBUF_INIT;
int fd;
@@ -867,7 +867,7 @@ static void save_head(const char *head)
static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
{
- const char *todo_file = git_path(SEQ_TODO_FILE);
+ const char *todo_file = git_path_unsafe(SEQ_TODO_FILE);
static struct lock_file todo_lock;
struct strbuf buf = STRBUF_INIT;
int fd;
@@ -888,7 +888,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
static void save_opts(struct replay_opts *opts)
{
- const char *opts_file = git_path(SEQ_OPTS_FILE);
+ const char *opts_file = git_path_unsafe(SEQ_OPTS_FILE);
if (opts->no_commit)
git_config_set_in_file(opts_file, "options.no-commit", "true");
@@ -969,7 +969,7 @@ static int pick_revisions(struct replay_opts *opts)
remove_sequencer_state(1);
return 0;
} else if (opts->subcommand == REPLAY_CONTINUE) {
- if (!file_exists(git_path(SEQ_TODO_FILE)))
+ if (!file_exists(git_path_unsafe(SEQ_TODO_FILE)))
goto error;
read_populate_opts(&opts);
read_populate_todo(&todo_list, opts);
diff --git a/cache.h b/cache.h
index 2e6ad360..7fb85445 100644
--- a/cache.h
+++ b/cache.h
@@ -662,7 +662,8 @@ extern char *git_pathdup(const char *fmt, ...)
/* Return a statically allocated filename matching the sha1 signature */
extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
-extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+extern char *git_path_unsafe(const char *fmt, ...)
+ __attribute__((format (printf, 1, 2)));
extern char *git_path_submodule(const char *path, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
diff --git a/contrib/examples/builtin-fetch--tool.c b/contrib/examples/builtin-fetch--tool.c
index 3140e405..ea18fdac 100644
--- a/contrib/examples/builtin-fetch--tool.c
+++ b/contrib/examples/builtin-fetch--tool.c
@@ -515,7 +515,7 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)
if (argc != 8)
return error("append-fetch-head takes 6 args");
- filename = git_path("FETCH_HEAD");
+ filename = git_path_unsafe("FETCH_HEAD");
fp = fopen(filename, "a");
if (!fp)
return error("cannot open %s: %s\n", filename, strerror(errno));
@@ -533,7 +533,7 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)
if (argc != 5)
return error("fetch-native-store takes 3 args");
- filename = git_path("FETCH_HEAD");
+ filename = git_path_unsafe("FETCH_HEAD");
fp = fopen(filename, "a");
if (!fp)
return error("cannot open %s: %s\n", filename, strerror(errno));
diff --git a/dir.c b/dir.c
index 6c0d7825..94662509 100644
--- a/dir.c
+++ b/dir.c
@@ -1224,7 +1224,7 @@ void setup_standard_excludes(struct dir_struct *dir)
const char *path;
dir->exclude_per_dir = ".gitignore";
- path = git_path("info/exclude");
+ path = git_path_unsafe("info/exclude");
if (!access(path, R_OK))
add_excludes_from_file(dir, path);
if (excludes_file && !access(excludes_file, R_OK))
diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c4..04bcd353 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -403,7 +403,7 @@ static void dump_marks_helper(FILE *, uintmax_t, struct mark_set *);
static void write_crash_report(const char *err)
{
- char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
+ char *loc = git_path_unsafe("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
FILE *rpt = fopen(loc, "w");
struct branch *b;
unsigned long lu;
diff --git a/http-backend.c b/http-backend.c
index 59ad7da6..7169e040 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -161,7 +161,7 @@ static void send_strbuf(const char *type, struct strbuf *buf)
static void send_local_file(const char *the_type, const char *name)
{
- const char *p = git_path("%s", name);
+ const char *p = git_path_unsafe("%s", name);
size_t buf_alloc = 8192;
char *buf = xmalloc(buf_alloc);
int fd;
diff --git a/notes-merge.c b/notes-merge.c
index e9e41993..0b49e8ad 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -275,35 +275,37 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
* Must establish NOTES_MERGE_WORKTREE.
* Abort if NOTES_MERGE_WORKTREE already exists
*/
- if (file_exists(git_path(NOTES_MERGE_WORKTREE))) {
+ if (file_exists(git_path_unsafe(NOTES_MERGE_WORKTREE))) {
if (advice_resolve_conflict)
die("You have not concluded your previous "
"notes merge (%s exists).\nPlease, use "
"'git notes merge --commit' or 'git notes "
"merge --abort' to commit/abort the "
"previous merge before you start a new "
- "notes merge.", git_path("NOTES_MERGE_*"));
+ "notes merge.",
+ git_path_unsafe("NOTES_MERGE_*"));
else
die("You have not concluded your notes merge "
- "(%s exists).", git_path("NOTES_MERGE_*"));
+ "(%s exists).",
+ git_path_unsafe("NOTES_MERGE_*"));
}
- if (safe_create_leading_directories(git_path(
+ if (safe_create_leading_directories(git_path_unsafe(
NOTES_MERGE_WORKTREE "/.test")))
die_errno("unable to create directory %s",
- git_path(NOTES_MERGE_WORKTREE));
+ git_path_unsafe(NOTES_MERGE_WORKTREE));
o->has_worktree = 1;
- } else if (!file_exists(git_path(NOTES_MERGE_WORKTREE)))
+ } else if (!file_exists(git_path_unsafe(NOTES_MERGE_WORKTREE)))
/* NOTES_MERGE_WORKTREE should already be established */
die("missing '%s'. This should not happen",
- git_path(NOTES_MERGE_WORKTREE));
+ git_path_unsafe(NOTES_MERGE_WORKTREE));
}
static void write_buf_to_worktree(const unsigned char *obj,
const char *buf, unsigned long size)
{
int fd;
- char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
+ char *path = git_path_unsafe(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
if (safe_create_leading_directories(path))
die_errno("unable to create directory for '%s'", path);
if (file_exists(path))
@@ -681,7 +683,7 @@ int notes_merge_commit(struct notes_merge_options *o,
* Finally store the new commit object SHA1 into 'result_sha1'.
*/
struct dir_struct dir;
- char *path = xstrdup(git_path(NOTES_MERGE_WORKTREE "/"));
+ char *path = git_pathdup(NOTES_MERGE_WORKTREE "/");
int path_len = strlen(path), i;
const char *msg = strstr(partial_commit->buffer, "\n\n");
@@ -731,7 +733,7 @@ int notes_merge_abort(struct notes_merge_options *o)
struct strbuf buf = STRBUF_INIT;
int ret;
- strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE));
+ strbuf_addstr(&buf, git_path_unsafe(NOTES_MERGE_WORKTREE));
OUTPUT(o, 3, "Removing notes merge worktree at %s", buf.buf);
ret = remove_dir_recursively(&buf, 0);
strbuf_release(&buf);
diff --git a/pack-refs.c b/pack-refs.c
index 23bbd00e..9557b063 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -86,7 +86,7 @@ static void try_remove_empty_parents(char *name)
if (q == p)
break;
*q = '\0';
- if (rmdir(git_path("%s", name)))
+ if (rmdir(git_path_unsafe("%s", name)))
break;
}
}
@@ -97,7 +97,7 @@ static void prune_ref(struct ref_to_prune *r)
struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
if (lock) {
- unlink_or_warn(git_path("%s", r->name));
+ unlink_or_warn(git_path_unsafe("%s", r->name));
unlock_ref(lock);
try_remove_empty_parents(r->name);
}
@@ -121,7 +121,7 @@ int pack_refs(unsigned int flags)
memset(&cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- fd = hold_lock_file_for_update(&packed, git_path("packed-refs"),
+ fd = hold_lock_file_for_update(&packed, git_path_unsafe("packed-refs"),
LOCK_DIE_ON_ERROR);
cbdata.refs_file = fdopen(fd, "w");
if (!cbdata.refs_file)
diff --git a/path.c b/path.c
index b6f71d10..0611b7be 100644
--- a/path.c
+++ b/path.c
@@ -101,7 +101,7 @@ char *mkpath(const char *fmt, ...)
return cleanup_path(pathname);
}
-char *git_path(const char *fmt, ...)
+char *git_path_unsafe(const char *fmt, ...)
{
const char *git_dir = get_git_dir();
char *pathname = get_pathname();
diff --git a/refs.c b/refs.c
index e69ba26b..e527c7b7 100644
--- a/refs.c
+++ b/refs.c
@@ -268,7 +268,7 @@ static struct ref_array *get_packed_refs(const char *submodule)
if (submodule)
packed_refs_file = git_path_submodule(submodule, "packed-refs");
else
- packed_refs_file = git_path("packed-refs");
+ packed_refs_file = git_path_unsafe("packed-refs");
f = fopen(packed_refs_file, "r");
if (f) {
read_packed_refs(f, &refs->packed);
@@ -288,7 +288,7 @@ static void get_ref_dir(const char *submodule, const char *base,
if (submodule)
path = git_path_submodule(submodule, "%s", base);
else
- path = git_path("%s", base);
+ path = git_path_unsafe("%s", base);
dir = opendir(path);
@@ -319,7 +319,7 @@ static void get_ref_dir(const char *submodule, const char *base,
memcpy(ref + baselen, de->d_name, namelen+1);
refdir = submodule
? git_path_submodule(submodule, "%s", ref)
- : git_path("%s", ref);
+ : git_path_unsafe("%s", ref);
if (stat(refdir, &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
@@ -1146,11 +1146,11 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
ref = resolve_ref(path, hash, 1, NULL);
if (!ref)
continue;
- if (!stat(git_path("logs/%s", path), &st) &&
+ if (!stat(git_path_unsafe("logs/%s", path), &st) &&
S_ISREG(st.st_mode))
it = path;
else if (strcmp(ref, path) &&
- !stat(git_path("logs/%s", ref), &st) &&
+ !stat(git_path_unsafe("logs/%s", ref), &st) &&
S_ISREG(st.st_mode))
it = ref;
else
@@ -1186,7 +1186,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
* it is normal for the empty directory 'foo'
* to remain.
*/
- ref_file = git_path("%s", orig_ref);
+ ref_file = git_path_unsafe("%s", orig_ref);
if (remove_empty_directories(ref_file)) {
last_errno = errno;
error("there are still refs under '%s'", orig_ref);
@@ -1223,7 +1223,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
}
lock->ref_name = xstrdup(ref);
lock->orig_ref_name = xstrdup(orig_ref);
- ref_file = git_path("%s", ref);
+ ref_file = git_path_unsafe("%s", ref);
if (missing)
lock->force_write = 1;
if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
@@ -1272,9 +1272,10 @@ static int repack_without_ref(const char *refname)
ref = search_ref_array(packed, refname);
if (ref == NULL)
return 0;
- fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
+ fd = hold_lock_file_for_update(&packlock,
+ git_path_unsafe("packed-refs"), 0);
if (fd < 0) {
- unable_to_lock_error(git_path("packed-refs"), errno);
+ unable_to_lock_error(git_path_unsafe("packed-refs"), errno);
return error("cannot delete '%s' from packed refs", refname);
}
@@ -1313,7 +1314,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
lock->lk->filename[i] = 0;
path = lock->lk->filename;
} else {
- path = git_path("%s", refname);
+ path = git_path_unsafe("%s", refname);
}
err = unlink_or_warn(path);
if (err && errno != ENOENT)
@@ -1328,7 +1329,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
*/
ret |= repack_without_ref(refname);
- unlink_or_warn(git_path("logs/%s", lock->ref_name));
+ unlink_or_warn(git_path_unsafe("logs/%s", lock->ref_name));
invalidate_ref_cache(NULL);
unlock_ref(lock);
return ret;
@@ -1349,7 +1350,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
int flag = 0, logmoved = 0;
struct ref_lock *lock;
struct stat loginfo;
- int log = !lstat(git_path("logs/%s", oldref), &loginfo);
+ int log = !lstat(git_path_unsafe("logs/%s", oldref), &loginfo);
const char *symref = NULL;
if (log && S_ISLNK(loginfo.st_mode))
@@ -1368,7 +1369,8 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
if (!is_refname_available(newref, oldref, get_loose_refs(NULL), 0))
return 1;
- if (log && rename(git_path("logs/%s", oldref), git_path(TMP_RENAMED_LOG)))
+ if (log && rename(git_path_unsafe("logs/%s", oldref),
+ git_path_unsafe(TMP_RENAMED_LOG)))
return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
oldref, strerror(errno));
@@ -1379,7 +1381,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
- if (remove_empty_directories(git_path("%s", newref))) {
+ if (remove_empty_directories(git_path_unsafe("%s", newref))) {
error("Directory not empty: %s", newref);
goto rollback;
}
@@ -1389,20 +1391,21 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
}
}
- if (log && safe_create_leading_directories(git_path("logs/%s", newref))) {
+ if (log && safe_create_leading_directories(git_path_unsafe("logs/%s", newref))) {
error("unable to create directory for %s", newref);
goto rollback;
}
retry:
- if (log && rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newref))) {
+ if (log && rename(git_path_unsafe(TMP_RENAMED_LOG),
+ git_path_unsafe("logs/%s", newref))) {
if (errno==EISDIR || errno==ENOTDIR) {
/*
* rename(a, b) when b is an existing
* directory ought to result in ISDIR, but
* Solaris 5.8 gives ENOTDIR. Sheesh.
*/
- if (remove_empty_directories(git_path("logs/%s", newref))) {
+ if (remove_empty_directories(git_path_unsafe("logs/%s", newref))) {
error("Directory not empty: logs/%s", newref);
goto rollback;
}
@@ -1444,11 +1447,13 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
log_all_ref_updates = flag;
rollbacklog:
- if (logmoved && rename(git_path("logs/%s", newref), git_path("logs/%s", oldref)))
+ if (logmoved && rename(git_path_unsafe("logs/%s", newref),
+ git_path_unsafe("logs/%s", oldref)))
error("unable to restore logfile %s from %s: %s",
oldref, newref, strerror(errno));
if (!logmoved && log &&
- rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldref)))
+ rename(git_path_unsafe(TMP_RENAMED_LOG),
+ git_path_unsafe("logs/%s", oldref)))
error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s",
oldref, strerror(errno));
@@ -1741,7 +1746,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
void *log_mapped;
size_t mapsz;
- logfile = git_path("logs/%s", ref);
+ logfile = git_path_unsafe("logs/%s", ref);
logfd = open(logfile, O_RDONLY, 0);
if (logfd < 0)
die_errno("Unable to read log '%s'", logfile);
@@ -1841,7 +1846,7 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long ofs,
struct strbuf sb = STRBUF_INIT;
int ret = 0;
- logfile = git_path("logs/%s", ref);
+ logfile = git_path_unsafe("logs/%s", ref);
logfp = fopen(logfile, "r");
if (!logfp)
return -1;
@@ -1899,7 +1904,7 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
{
- DIR *dir = opendir(git_path("logs/%s", base));
+ DIR *dir = opendir(git_path_unsafe("logs/%s", base));
int retval = 0;
if (dir) {
@@ -1923,7 +1928,7 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
if (has_extension(de->d_name, ".lock"))
continue;
memcpy(log + baselen, de->d_name, namelen+1);
- if (stat(git_path("logs/%s", log), &st) < 0)
+ if (stat(git_path_unsafe("logs/%s", log), &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
retval = do_for_each_reflog(log, fn, cb_data);
diff --git a/remote.c b/remote.c
index e2ef9911..bb85b326 100644
--- a/remote.c
+++ b/remote.c
@@ -224,7 +224,7 @@ static void add_instead_of(struct rewrite *rewrite, const char *instead_of)
static void read_remotes_file(struct remote *remote)
{
- FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
+ FILE *f = fopen(git_path_unsafe("remotes/%s", remote->name), "r");
if (!f)
return;
@@ -275,7 +275,7 @@ static void read_branches_file(struct remote *remote)
char *frag;
struct strbuf branch = STRBUF_INIT;
int n = slash ? slash - remote->name : 1000;
- FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
+ FILE *f = fopen(git_path_unsafe("branches/%.*s", n, remote->name), "r");
char *s, *p;
int len;
diff --git a/rerere.c b/rerere.c
index dcb525a4..61f60701 100644
--- a/rerere.c
+++ b/rerere.c
@@ -22,7 +22,7 @@ static char *merge_rr_path;
const char *rerere_path(const char *hex, const char *file)
{
- return git_path("rr-cache/%s/%s", hex, file);
+ return git_path_unsafe("rr-cache/%s/%s", hex, file);
}
int has_rerere_resolution(const char *hex)
@@ -524,7 +524,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
continue;
hex = xstrdup(sha1_to_hex(sha1));
string_list_insert(rr, path)->util = hex;
- if (mkdir(git_path("rr-cache/%s", hex), 0755))
+ if (mkdir(git_path_unsafe("rr-cache/%s", hex), 0755))
continue;
handle_file(path, NULL, rerere_path(hex, "preimage"));
fprintf(stderr, "Recorded preimage for '%s'\n", path);
@@ -591,7 +591,7 @@ static int is_rerere_enabled(void)
if (!rerere_enabled)
return 0;
- rr_cache = git_path("rr-cache");
+ rr_cache = git_path_unsafe("rr-cache");
rr_cache_exists = is_directory(rr_cache);
if (rerere_enabled < 0)
return rr_cache_exists;
@@ -695,7 +695,7 @@ static void unlink_rr_item(const char *name)
unlink(rerere_path(name, "thisimage"));
unlink(rerere_path(name, "preimage"));
unlink(rerere_path(name, "postimage"));
- rmdir(git_path("rr-cache/%s", name));
+ rmdir(git_path_unsafe("rr-cache/%s", name));
}
struct rerere_gc_config_cb {
@@ -726,7 +726,7 @@ void rerere_gc(struct string_list *rr)
struct rerere_gc_config_cb cf = { 15, 60 };
git_config(git_rerere_gc_config, &cf);
- dir = opendir(git_path("rr-cache"));
+ dir = opendir(git_path_unsafe("rr-cache"));
if (!dir)
die_errno("unable to open rr-cache directory");
while ((e = readdir(dir))) {
@@ -760,5 +760,5 @@ void rerere_clear(struct string_list *merge_rr)
if (!has_rerere_resolution(name))
unlink_rr_item(name);
}
- unlink_or_warn(git_path("MERGE_RR"));
+ unlink_or_warn(git_path_unsafe("MERGE_RR"));
}
diff --git a/run-command.c b/run-command.c
index 1c510438..598e41dd 100644
--- a/run-command.c
+++ b/run-command.c
@@ -612,11 +612,11 @@ int run_hook(const char *index_file, const char *name, ...)
va_list args;
int ret;
- if (access(git_path("hooks/%s", name), X_OK) < 0)
+ if (access(git_path_unsafe("hooks/%s", name), X_OK) < 0)
return 0;
va_start(args, name);
- argv_array_push(&argv, git_path("hooks/%s", name));
+ argv_array_push(&argv, git_path_unsafe("hooks/%s", name));
while ((p = va_arg(args, const char *)))
argv_array_push(&argv, p);
va_end(args);
diff --git a/sequencer.c b/sequencer.c
index bc2c046a..2e29152c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -8,10 +8,10 @@ void remove_sequencer_state(int aggressive)
struct strbuf seq_dir = STRBUF_INIT;
struct strbuf seq_old_dir = STRBUF_INIT;
- strbuf_addf(&seq_dir, "%s", git_path(SEQ_DIR));
- strbuf_addf(&seq_old_dir, "%s", git_path(SEQ_OLD_DIR));
+ strbuf_addf(&seq_dir, "%s", git_path_unsafe(SEQ_DIR));
+ strbuf_addf(&seq_old_dir, "%s", git_path_unsafe(SEQ_OLD_DIR));
remove_dir_recursively(&seq_old_dir, 0);
- rename(git_path(SEQ_DIR), git_path(SEQ_OLD_DIR));
+ rename(git_path_unsafe(SEQ_DIR), git_path_unsafe(SEQ_OLD_DIR));
if (aggressive)
remove_dir_recursively(&seq_old_dir, 0);
strbuf_release(&seq_dir);
diff --git a/server-info.c b/server-info.c
index 9ec744e9..348a3447 100644
--- a/server-info.c
+++ b/server-info.c
@@ -243,7 +243,7 @@ int update_server_info(int force)
errs = errs | update_info_packs(force);
/* remove leftover rev-cache file if there is any */
- unlink_or_warn(git_path("info/rev-cache"));
+ unlink_or_warn(git_path_unsafe("info/rev-cache"));
return errs;
}
diff --git a/sha1_file.c b/sha1_file.c
index 86705bc9..ba7eca89 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -382,7 +382,7 @@ static void read_info_alternates(const char * relative_base, int depth)
void add_to_alternates_file(const char *reference)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int fd = hold_lock_file_for_append(lock, git_path("objects/info/alternates"), LOCK_DIE_ON_ERROR);
+ int fd = hold_lock_file_for_append(lock, git_path_unsafe("objects/info/alternates"), LOCK_DIE_ON_ERROR);
char *alt = mkpath("%s\n", reference);
write_or_die(fd, alt, strlen(alt));
if (commit_lock_file(lock))
@@ -2706,7 +2706,7 @@ static int index_stream(unsigned char *sha1, int fd, size_t size,
int len, tmpfd;
strbuf_addstr(&export_marks, "--export-marks=");
- strbuf_addstr(&export_marks, git_path("hashstream_XXXXXX"));
+ strbuf_addstr(&export_marks, git_path_unsafe("hashstream_XXXXXX"));
tmpfile = export_marks.buf + strlen("--export-marks=");
tmpfd = git_mkstemp_mode(tmpfile, 0600);
if (tmpfd < 0)
diff --git a/shallow.c b/shallow.c
index a0363dea..d721397a 100644
--- a/shallow.c
+++ b/shallow.c
@@ -25,7 +25,7 @@ int is_repository_shallow(void)
if (is_shallow >= 0)
return is_shallow;
- fp = fopen(git_path("shallow"), "r");
+ fp = fopen(git_path_unsafe("shallow"), "r");
if (!fp) {
is_shallow = 0;
return is_shallow;
diff --git a/transport.c b/transport.c
index 51814b5d..cc0ca04c 100644
--- a/transport.c
+++ b/transport.c
@@ -203,7 +203,7 @@ static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)
/* copy the refs to the temporary directory */
- strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
+ strbuf_addstr(&temp_dir, git_path_unsafe("rsync-refs-XXXXXX"));
if (!mkdtemp(temp_dir.buf))
die_errno ("Could not make temporary directory");
temp_dir_len = temp_dir.len;
@@ -366,7 +366,7 @@ static int rsync_transport_push(struct transport *transport,
/* copy the refs to the temporary directory; they could be packed. */
- strbuf_addstr(&temp_dir, git_path("rsync-refs-XXXXXX"));
+ strbuf_addstr(&temp_dir, git_path_unsafe("rsync-refs-XXXXXX"));
if (!mkdtemp(temp_dir.buf))
die_errno ("Could not make temporary directory");
strbuf_addch(&temp_dir, '/');
diff --git a/unpack-trees.c b/unpack-trees.c
index 8282f5e5..44f408b8 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1010,7 +1010,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
if (!core_apply_sparse_checkout || !o->update)
o->skip_sparse_checkout = 1;
if (!o->skip_sparse_checkout) {
- if (add_excludes_from_file_to_list(git_path("info/sparse-checkout"), "", 0, NULL, &el, 0) < 0)
+ if (add_excludes_from_file_to_list(git_path_unsafe("info/sparse-checkout"), "", 0, NULL, &el, 0) < 0)
o->skip_sparse_checkout = 1;
else
o->el = ⪙
--
1.7.8.rc0
^ permalink raw reply related
* are X-like merges bad?
From: Ilya Basin @ 2011-11-16 8:20 UTC (permalink / raw)
To: git
When you merge branch A with branch B, you can then fast forward
branch B to the merge commit. git merge even does this automatically.
Such commits have 2 parents and 2 children.
On both branches 'git log --first-parent' shows the same history
before the merge.
Is it true that in this case you can't filter commits by branch name
in 'git log'?
I'm thinking of using 'git merge --no-ff' since now.
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Nguyen Thai Ngoc Duy @ 2011-11-16 8:37 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <20111116075955.GB13706@elie.hsd1.il.comcast.net>
2011/11/16 Jonathan Nieder <jrnieder@gmail.com>:
> Ramkumar Ramachandra wrote:
>> Junio C Hamano wrote:
>
>>> Or perhaps http://thread.gmane.org/gmane.comp.version-control.git/184963/focus=185436
>>
>> I noticed that sha1_to_hex() also operates like this. The
>> resolve_ref() function is really important, but using the same
>> technique for these tiny functions is probably an overkill
>
> I don't follow. Do you mean that not being confusing is overkill,
> because the function is small that no one will bother to look up the
> right semantics? Wait, that sentence didn't come out the way I
> wanted. ;-)
>
> Jokes aside, here's a rough series to do the git_path ->
> git_path_unsafe renaming. While writing it, I noticed a couple of
> bugs, hence the two patches before the last one. Patch 2 is the more
> interesting one.
Or perhaps
- kill git_path(const char *fmt, ...) in favor of git_pathdup() companion
- git_path(const char *path) maintains a small hash table to keep
track of all returned strings based with "path" as key.
Out of 142 git_path() calls in my tree, 97 of them are in form
git_path("some static string"). git_path() could learn to keep track
of all generated strings while keep it convenient to use. I suspect
with some macro magic, we can keep track of generated strings without
a hash table.
--
Duy
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Nguyen Thai Ngoc Duy @ 2011-11-16 8:42 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <CACsJy8Di3ZrPdXh1Jf=PbLYRWwx-TEV78NzUukwaxA0xW=rSNg@mail.gmail.com>
On Wed, Nov 16, 2011 at 3:37 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> I suspect
> with some macro magic, we can keep track of generated strings without
> a hash table.
I misremembered. There is an operator to convert symbol to string, not
the other way around.
--
Duy
^ permalink raw reply
* Re: Git 1.7.5 problem with HTTPS
From: Dmitry Smirnov @ 2011-11-16 8:51 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Junio C Hamano, Shawn Pearce, git
In-Reply-To: <CALUzUxrM8o1uahQgSFUuvZ0mSPxG_zVQ9awOantRM2A8kkbbtA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 746 bytes --]
Thanks.
I had collected two logs (for clone and ls-remote, attached).
Unfortunately, I cannot see, why problem occurs. The only indication is
* Connection #0 seems to be dead!
Is it possible that curl sends the request in plain text?
And according to tcpdump, why git/curl sends the request before Server Hello?
2011/11/16 Tay Ray Chuan <rctay89@gmail.com>:
> On Wed, Nov 16, 2011 at 3:11 PM, Dmitry Smirnov <divis1969@gmail.com> wrote:
>> What if problem is caused by curl or TLS lib (libcurl-gnutls?) which
>> is used by my git? Is there any to log something from git-remote-https
>> ?
>
> You can run git with GIT_CURL_VERBOSE set, like this
>
> GIT_CURL_VERBOSE=1 git ls-remote ...
>
> --
> Cheers,
> Ray Chuan
>
[-- Attachment #2: clone.log --]
[-- Type: text/x-log, Size: 1899 bytes --]
dsmirnov@dsmirnov-ubuntu2:~/projects/tmp$ GIT_CURL_VERBOSE=1 GIT_TRACE=true git clone --verbose https://git.kernel.org/pub/scm/git/git.git
trace: built-in: git 'clone' '--verbose' 'https://git.kernel.org/pub/scm/git/git.git'
Cloning into git...
trace: run_command: 'git-remote-https' 'origin' 'https://git.kernel.org/pub/scm/git/git.git'
* Couldn't find host git.kernel.org in the .netrc file; using defaults
* About to connect() to proxy proxy.yyyyy.yy port 3128 (#0)
* Trying Y.Y.Y.Y... * Connected to proxy.yyyyy.yy (Y.Y.Y.Y) port 3128 (#0)
* Establish HTTP proxy tunnel to git.kernel.org:443
> CONNECT git.kernel.org:443 HTTP/1.1
Host: git.kernel.org:443
User-Agent: git/1.7.5.4
Proxy-Connection: Keep-Alive
Pragma: no-cache
< HTTP/1.0 200 Connection established
<
* Proxy replied OK to CONNECT request
* found 157 certificates in /etc/ssl/certs/ca-certificates.crt
> GET /pub/scm/git/git.git/info/refs?service=git-upload-pack HTTP/1.1
User-Agent: git/1.7.5.4
Host: git.kernel.org
Accept: */*
Pragma: no-cache
* Connection #0 to host proxy.yyyyy.yy left intact
* Couldn't find host git.kernel.org in the .netrc file; using defaults
* Connection #0 seems to be dead!
* Closing connection #0
* About to connect() to proxy proxy.yyyyy.yy port 3128 (#0)
* Trying Y.Y.Y.Y... * Connected to proxy.yyyyy.yy (Y.Y.Y.Y) port 3128 (#0)
* Establish HTTP proxy tunnel to git.kernel.org:443
> CONNECT git.kernel.org:443 HTTP/1.1
Host: git.kernel.org:443
User-Agent: git/1.7.5.4
Proxy-Connection: Keep-Alive
Pragma: no-cache
< HTTP/1.0 200 Connection established
<
* Proxy replied OK to CONNECT request
* found 157 certificates in /etc/ssl/certs/ca-certificates.crt
> GET /pub/scm/git/git.git/HEAD HTTP/1.1
User-Agent: git/1.7.5.4
Host: git.kernel.org
Accept: */*
Pragma: no-cache
* Connection #0 to host proxy.yyyyy.yy left intact
warning: remote HEAD refers to nonexistent ref, unable to checkout.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: ls-remote.log --]
[-- Type: text/x-log; name="ls-remote.log", Size: 2365 bytes --]
dsmirnov@dsmirnov-ubuntu2:~/projects/tmp$ GIT_CURL_VERBOSE=1 GIT_TRACE=true git ls-remote https://git.kernel.org/pub/scm/git/git.git
trace: built-in: git 'ls-remote' 'https://git.kernel.org/pub/scm/git/git.git'
trace: run_command: 'git-remote-https' 'https://git.kernel.org/pub/scm/git/git.git' 'https://git.kernel.org/pub/scm/git/git.git'
* Couldn't find host git.kernel.org in the .netrc file; using defaults
* About to connect() to proxy proxy.yyyyy.yy port 3128 (#0)
* Trying Y.Y.Y.Y... * Connected to proxy.yyyyy.yy (Y.Y.Y.Y) port 3128 (#0)
* Establish HTTP proxy tunnel to git.kernel.org:443
> CONNECT git.kernel.org:443 HTTP/1.1
Host: git.kernel.org:443
User-Agent: git/1.7.5.4
Proxy-Connection: Keep-Alive
Pragma: no-cache
< HTTP/1.0 200 Connection established
<
* Proxy replied OK to CONNECT request
* found 157 certificates in /etc/ssl/certs/ca-certificates.crt
> GET /pub/scm/git/git.git/info/refs?service=git-upload-pack HTTP/1.1
User-Agent: git/1.7.5.4
Host: git.kernel.org
Accept: */*
Pragma: no-cache
* Connection #0 to host proxy.yyyyy.yy left intact
* Couldn't find host git.kernel.org in the .netrc file; using defaults
* Connection #0 seems to be dead!
* Closing connection #0
* About to connect() to proxy proxy.yyyyy.yy port 3128 (#0)
* Trying Y.Y.Y.Y... * Connected to proxy.yyyyy.yy (Y.Y.Y.Y) port 3128 (#0)
* Establish HTTP proxy tunnel to git.kernel.org:443
> CONNECT git.kernel.org:443 HTTP/1.1
Host: git.kernel.org:443
User-Agent: git/1.7.5.4
Proxy-Connection: Keep-Alive
Pragma: no-cache
< HTTP/1.0 200 Connection established
<
* Proxy replied OK to CONNECT request
* found 157 certificates in /etc/ssl/certs/ca-certificates.crt
> GET /pub/scm/git/git.git/HEAD HTTP/1.1
User-Agent: git/1.7.5.4
Host: git.kernel.org
Accept: */*
Pragma: no-cache
* Connection #0 to host proxy.yyyyy.yy left intact
0000000000000000000000000000000000000000 \x03\x01
0000000000000000000000000000000000000000 Cape
0000000000000000000000000000000000000000 \x06\x03U\x04\x06\x13\x02US1\x130\x11\x06\x03U\x13
ca00000000000000000000000000000000000000 \x06\x03U\x04\x06\x13\x02US1\x130\x11\x06\x03U\x13
\x01\x01\x010000000000000000000000000000000000000 *�H�
\x01\x01\x010000000000000000000000000000000000000 *�H�
0000000000000000000000000000000000000000
0000000000000000000000000000000000000000 (��v�K\�M�d�����H���o\x10o\x19��\x1dD����]��g�\x02I\x01�Fo�1�����_\x02^U#��2o�����
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Ramkumar Ramachandra @ 2011-11-16 8:51 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Git List, Nguyễn Thái Ngọc
In-Reply-To: <20111116075955.GB13706@elie.hsd1.il.comcast.net>
Hi Jonathan,
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>> Junio C Hamano wrote:
>
>>> Or perhaps http://thread.gmane.org/gmane.comp.version-control.git/184963/focus=185436
>>
>> I noticed that sha1_to_hex() also operates like this. The
>> resolve_ref() function is really important, but using the same
>> technique for these tiny functions is probably an overkill
>
> I don't follow. Do you mean that not being confusing is overkill,
> because the function is small that no one will bother to look up the
> right semantics? Wait, that sentence didn't come out the way I
> wanted. ;-)
I meant overkill in terms of the work required and code churn.
Ofcourse, I'd have been more than happy to see it being implemented-
and you've actually done it now! :) Nguyễn has a more fancy solution,
though I'm quite happy with this as it is.
Finally, for all the times I've fumbled in git_path() usage:
Liked-by: Ramkumar Ramachandra <artagnon@gmail.com>
Thanks for working on this.
-- Ram
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Jonathan Nieder @ 2011-11-16 8:59 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <CACsJy8Di3ZrPdXh1Jf=PbLYRWwx-TEV78NzUukwaxA0xW=rSNg@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> Or perhaps
[...]
> - git_path(const char *path) maintains a small hash table to keep
> track of all returned strings based with "path" as key.
>
> Out of 142 git_path() calls in my tree, 97 of them are in form
> git_path("some static string").
The main bit I dislike about patch 3/3 is that constructs like
'unlink(git_path("MERGE_HEAD"));' are not actually unsafe, unless they
happen to sit in the middle of an unsafe
const char *filename = git_path(foo);
int fd;
call_a_function_i_dont_control();
fd = open(filename, O_CREAT|O_WRONLY|O_TRUNC, 0600);
sequence. Lacks that feeling of truth in advertising. And on the
other hand that this doesn't help with thread-safety at all.
I think if I ran the world, the fundamental operation would be
strbuf_addpath(). Unlike git_pathdup(), this lets callers avoid some
allocation churn if they are in the middle of a loop.
^ permalink raw reply
* Re: Git 1.7.5 problem with HTTPS
From: Daniel Stenberg @ 2011-11-16 9:13 UTC (permalink / raw)
To: Dmitry Smirnov; +Cc: Tay Ray Chuan, Junio C Hamano, Shawn Pearce, git
In-Reply-To: <CACf55T5cp1ko45DCufcRXm=EeZd1-x+aYasvbzjDXkQH31u5VA@mail.gmail.com>
On Wed, 16 Nov 2011, Dmitry Smirnov wrote:
> Unfortunately, I cannot see, why problem occurs. The only indication is
> * Connection #0 seems to be dead!
That means libcurl wanted to re-use an existing connection, but it seems to
have died in the mean time and therefore it has to create a new one and
reconnect instead. I suppose that is the first indication that something isn't
quite right.
> Is it possible that curl sends the request in plain text?
I'd say that isn't very likely and you could easily snoop on the network to
figure that out for sure.
> And according to tcpdump, why git/curl sends the request before Server
> Hello?
curl will send the HTTP request once the TLS negotiation has completed as told
by the TLS library. I believe you said you're using GnuTLS, are you using a
recent version?
This is not a transfer layer (curl/HTTPS) bug I recognize, but I can of course
not rule out that there's a bug somewhere in there!
--
/ daniel.haxx.se
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Nguyen Thai Ngoc Duy @ 2011-11-16 9:31 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <20111116085944.GA18781@elie.hsd1.il.comcast.net>
On Wed, Nov 16, 2011 at 3:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyen Thai Ngoc Duy wrote:
>
>> Or perhaps
> [...]
>> - git_path(const char *path) maintains a small hash table to keep
>> track of all returned strings based with "path" as key.
>>
>> Out of 142 git_path() calls in my tree, 97 of them are in form
>> git_path("some static string").
>
> The main bit I dislike about patch 3/3 is that constructs like
> 'unlink(git_path("MERGE_HEAD"));' are not actually unsafe
Well, we can create wrappers (e.g. repo_unlink(const char *) that
calls git_path internally). According to grep/sed these functions are
used in form xxx(git_path(xxx))
16 unlink
8 file_exists
7 stat
6 fopen
5 rename
5 open
4 unlink_or_warn
3 safe_create_dir
3 adjust_shared_perm
3 access
2 xstrdup
2 safe_create_leading_directories
2 rmdir
2 remove_empty_directories
2 opendir
1 unable_to_lock_error
1 read_attr_from_file
1 mkdir
1 lstat
1 launch_editor
1 add_excludes_from_file_to_list
By creating wrappers for unlink, file_exists, stat, fopen, rename and
open we can safely avoid git_pathdup()/free() in 42 places.
--
Duy
^ permalink raw reply
* Re: Git 1.7.5 problem with HTTPS
From: Dmitry Smirnov @ 2011-11-16 10:10 UTC (permalink / raw)
To: Daniel Stenberg; +Cc: Tay Ray Chuan, Junio C Hamano, Shawn Pearce, git
In-Reply-To: <alpine.DEB.2.00.1111161008430.19479@tvnag.unkk.fr>
> I'd say that isn't very likely and you could easily snoop on the network to> figure that out for sure
In the very first message I wrote that there is strange tcpdump record:
21 3.815135 X.X.X.X Y.Y.Y.Y TLSv1 215 Ignored Unknown Record
In this record there is some binary dump followed by an uncripted text:
GET /pub/scm/git/git.git/info/refs?service=git-upload-pack HTTP/1.1
User-Agent: git/1.7.5.4 Host: git.kernel.org Accept: */* Pragma:
no-cache
This packet is recorded before negotiation complete, so I'm wondering
who is guilty: git or curl?
What Git is providing to libcurl? Can I log it?
> curl will send the HTTP request once the TLS negotiation has completed as
> told by the TLS library. I believe you said you're using GnuTLS, are you
> using a recent version?
I'm using the version that comes with Ubuntu 11.10.
^ permalink raw reply
* Re: [PATCH] i18n: add infrastructure for translating Git with gettext
From: Ævar Arnfjörð Bjarmason @ 2011-11-16 10:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jakub Narebski, Jeff Epler, Jonathan Nieder, Johannes Sixt,
Erik Faye-Lund, Thomas Rast, Peter Krefting
In-Reply-To: <7vlirhx14x.fsf@alter.siamese.dyndns.org>
On Tue, Nov 15, 2011 at 09:04, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Change the skeleton implementation of i18n in Git to one that can show
>> localized strings to users for our C, Shell and Perl programs using
>> either GNU libint or the Solaris gettext implementation.
>
> Happy ;-).
>
> Isn't it libintl, though?
>
>> This new internationalization support is enabled by default. If
>> gettext isn't available, or if Git is compiled with
>> NO_GETTEXT=YesPlease, Git fall back on its current behavior of showing
>
> s/fall/falls/;
I'll fix both of these speling errurs in a new version of the
patch.
But I won't send it for at least a few days. I don't think there's
much point as we're in no rush to apply this, and it might as well sit
on-list for a few days to accumulate more review.
>> This change is somewhat large because as well as adding a C, Shell and
>> Perl i18n interface we're adding a lot of tests for them, and for
>> those tests to work we need a skeleton PO file to actually test
>> translations.
>
> You _could_ split it up this way, I think, if you really wanted to:
>
> * The mechanism, i.e. integration with libintl and gettext, without any
> translated strings but with the .pot file, with tests to make sure in a
> locale that is missing *.mo files for it, we get the default output;
>
> * Sample translation for is_IS locale, with tests to make sure that we
> get translated output in a locale that has *.mo files for it.
>
> But I am not sure if that is better.
I think it's better to keep it in one large patch. The changes to the
core code are pretty minimal, most of what it's adding is new stuff
that's only ever used if i18n itself is used.
>> diff --git a/.gitignore b/.gitignore
>> index 8572c8c..c47f3a8 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -224,3 +224,4 @@
>> *.pdb
>> /Debug/
>> /Release/
>> +/share/
>
> I somehow find it distasteful that this is overly wide; same with the
> change in Makefile to "rm -rf share/". I suspect it wouldn't be the case
> if it were limited to share/locale/ or something.
>
> But perhaps we would never ship anything in share/ and things in there
> will always come from elsewhere.
>
>> diff --git a/Makefile b/Makefile
>> index ee34eab..896f5fd 100644
>> --- a/Makefile
>> +++ b/Makefile
>> ...
>> @@ -2435,6 +2507,7 @@ clean:
>> $(RM) $(TEST_PROGRAMS)
>> $(RM) -r bin-wrappers
>> $(RM) -r $(dep_dirs)
>> + $(RM) -r po/git.pot share/
>
> It seems "distclean" tells us to clean po/git.pot, which hints at me that
> normal "clean" shouldn't?
I'm not quite sure what we should do with it.
Here's what we do with the sharedir currently:
* It's used as the gitwebdir already, and is used also as the
localedir with this patch:
sharedir = $(prefix)/share
gitwebdir = $(sharedir)/gitweb
localedir = $(sharedir)/locale
* Even then I can't find the option that makes gitweb put something
in it, after a "make" with this patch:
$ find share -type f
share/locale/is/LC_MESSAGES/git.mo
* It *is* important that we have this under a
$SOMETHING/is/LC_MESSAGES/git.mo structure, since we set
GIT_TEXTDOMAINDIR to $SOMETHING, and the gettext library will only
understand something in this layout.
But that $SOMETHING doesn't have to be $ROOT/share, it could be
e.g. $ROOT/po/generated, which would make it not conflict with the
gitwebdir.
* Yeah, we shouldn't remove po/git.pot in "clean" as well as
"distclean". I missed that it had been added in 92a684b916 while
applying this patch.
^ permalink raw reply
* Re: Git 1.7.5 problem with HTTPS
From: Haitao Li @ 2011-11-16 10:32 UTC (permalink / raw)
To: Dmitry Smirnov; +Cc: git
In-Reply-To: <CACf55T6BGds_D=nbb8G=m+Jwr+bHFruCs-Q0+FOO+WXitXEJ-g@mail.gmail.com>
> I was also considering that the problem is caused by proxy. But when I
> tried to clone the same git source from another host via the same
> proxy, it works pretty good. The difference is the git version: on the
> first host it is 1.7.5.4 (comes with Ubuntu 11.10), on the second -
> 1.7.0.4
The proxy may have some impact.
I see exactly the same error only behind a proxy on my laptop running
Ubuntu 11.10 with libgnutls26/2.10.5-1ubuntu3. The same laptop works fine
at home without proxy.
I have another machine (Ubuntu 11.04 git/1.7.4.1 libgnutls26/2.8.6-1ubuntu2)
works fine behind the same proxy.
^ permalink raw reply
* Re: [PATCH] i18n: add infrastructure for translating Git with gettext
From: Jonathan Nieder @ 2011-11-16 10:33 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano, git, Jakub Narebski, Jeff Epler, Johannes Sixt,
Erik Faye-Lund, Thomas Rast, Peter Krefting
In-Reply-To: <CACBZZX4nypBW1agNw6NrC-7LBWbjZ1ycgpn-zvBsg0x4EDBD0g@mail.gmail.com>
Ævar Arnfjörð Bjarmason wrote:
> Here's what we do with the sharedir currently:
>
> * It's used as the gitwebdir already, and is used also as the
> localedir with this patch:
>
> sharedir = $(prefix)/share
> gitwebdir = $(sharedir)/gitweb
> localedir = $(sharedir)/locale
This has $(prefix)/ (e.g., /usr/) at the start.
> * Even then I can't find the option that makes gitweb put something
> in it, after a "make" with this patch:
>
> $ find share -type f
> share/locale/is/LC_MESSAGES/git.mo
This does not.
> * It *is* important that we have this under a
> $SOMETHING/is/LC_MESSAGES/git.mo structure, since we set
> GIT_TEXTDOMAINDIR to $SOMETHING, and the gettext library will only
> understand something in this layout.
>
> But that $SOMETHING doesn't have to be $ROOT/share, it could be
> e.g. $ROOT/po/generated, which would make it not conflict with the
> gitwebdir.
Yeah, makes sense. bin-wrappers/git has to get its locales from
somewhere.
^ permalink raw reply
* Monotone to git mirroring ... how to do updates
From: Matěj Cepl @ 2011-11-16 10:49 UTC (permalink / raw)
To: git
Hi,
I am trying to make script to be run from crontab which would
periodically mirror pidgin monotone repo to my git one (using
https://github.com/felipec/pidgin-git-import).
The main script in this repo is import:
#!/bin/sh
export GIT_DIR=pidgin.git
git init
git_marks="marks-git.txt"
mtn_marks="marks-mtn.txt"
touch $git_marks $mtn_marks
mtn --db pidgin.mtn pull
mtn git_export --db pidgin.mtn --authors-file=authors_map.txt \
--branches-file=branches_map.txt \
--refs=revs --import-marks=$mtn_marks --export-marks=$mtn_marks \
--use-one-changelog | \
git fast-import --import-marks=$git_marks --export-marks=$git_marks
I can see what this script does on the first run (when creating new git
repo), but what it does when I try to update with it already existing
repo from updated pidgin.mtn database? Where I can see the new changes?
Probably remote branches should be updated, right? (yes, I don't
understand well what actually git fast-import does, that's the problem).
Also, if I get eventually new changes to the remote branches on the git
repo, how can I update (preferably by one command) 140+ branches at
once? Or do I have to do something in the tune of?
for remote_branch in $(git branches -r) ; do
local_branch=$(echo $remote_branch |sed -e 's/origin\///')
git checkout $local_branch
git merge $remote_branch
done
Thank you in advance for any ideas,
Matěj
^ permalink raw reply
* Re: [PATCH] git-show-ref: fix escaping in asciidoc source
From: Michael Haggerty @ 2011-11-16 11:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <4EC34FA5.2020809@alum.mit.edu>
On 11/16/2011 06:52 AM, Michael Haggerty wrote:
> On 11/15/2011 08:16 PM, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>> Did this one fall through the cracks? I don't see it in your tree.
>>
>> Yeah, I was wondering if we can have a concise description in what context
>> any "^" must be spelled as {caret} and what other context "^" can be
>> spelled literally, and possibly which versions of AsciiDoc toolchain have
>> this issue [*1*]. Without a clear guideline, people may unknowingly use
>> literal "^" to new paragraphs, or perhaps worse yet, spell {caret} that
>> end up being shown literally.
>>
>> Since I didn't find a clear pattern other than that "^" can and should be
>> literally given in a literal paragraph (i.e. an indented paragraph or
>> inside a listing/literal block that shows program examples), I was meaning
>> to ask you if you knew the rules better than I did, and I stopped there,
>> forgetting to follow through.
>
> I didn't know anything about asciidoc, and just tried to fix it using a
> bit of cargo-cult programming. [...]
>
> I can't believe I spent my whole morning on this :-(
There are a couple of FAQ entries on the asciidoc site that are relevant
(excerpted; click on the links in the footnotes for the full text):
====================================================================
35 [1]. How can I place a backslash character in front of an attribute
reference without escaping the reference?
Use the predefined {backslash} attribute reference instead of an actual
backslash [...]
36 [2]. How can I escape AsciiDoc markup?
Most AsciiDoc inline elements can be suppressed by preceding them with a
backslash character. These elements include: [...] But there are
exceptions — see the next question.
37 [3]. Some elements can’t be escaped with a single backslash
There are a number of exceptions to the usual single backslash rule —
mostly relating to URL macros that have two syntaxes or quoting
ambiguity. Here are some non-standard escape examples: [...] A
work-around for difficult cases is to side-step the problem using the
pass:[] passthrough inline macro.
Note Escaping is unnecessary inside inline literal passthroughs
(backtick quoted text).
51 [4]. How can I selectively disable a quoted text substitution?
Omitting the tag name will disable quoting. For example, if you don’t
want superscripts or subscripts then put the following in a custom
configuration file or edit the global asciidoc.conf configuration file:
[quotes]
^=
~=
Alternatively you can set the configuration entries from within your
document, the above examples are equivalent to:
:quotes.^:
:quotes.~:
====================================================================
Given that the git documentation uses lots of "^" and "~" but probably
no subscripting or superscripting, it seems like the suggestion in FAQ
entry 51 would be helpful. But unfortunately it chokes version 8.5.2 of
asciidoc, which is what I have installed. So it is probably too new to
be appropriate for git use.
I also did a check to see whether other sub/superscripts are present in
the git documentation:
make all doc
find Documentation -type f -name '*.html' -print0 |
xargs -0 grep -nE -e '<su[bp]>'
The only hit was the one under discussion, in git-show-ref.html. (This
is no check that "^" and "~" are always quoted, but only that they don't
appear unquoted in pairs.)
Michael
[1]
http://www.methods.co.nz/asciidoc/faq.html#_how_can_i_place_a_backslash_character_in_front_of_an_attribute_reference_without_escaping_the_reference
[2]
http://www.methods.co.nz/asciidoc/faq.html#_how_can_i_escape_asciidoc_markup
[3]
http://www.methods.co.nz/asciidoc/faq.html#_some_elements_can_8217_t_be_escaped_with_a_single_backslash
[4]
http://www.methods.co.nz/asciidoc/faq.html#_how_can_i_selectively_disable_a_quoted_text_substitution
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* [PATCH] apply: squash consecutive slashes with p_value > 0
From: Robie Basak @ 2011-11-16 12:04 UTC (permalink / raw)
To: git
"patch" works with -p1 and diffs in the following form:
--- foo.orig//bar
+++ foo//bar
...
patch(1) says that "A sequence of one or more adjacent slashes is
counted as a single slash."
This patch amends git-apply's filename parsing to behave in the same
way, eliminating both '/'s with -p1 in this example.
Test case included.
Signed-off-by: Robie Basak <robie.basak@canonical.com>
---
builtin/apply.c | 8 ++++++--
t/t4153-apply-doubleslash.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
create mode 100755 t/t4153-apply-doubleslash.sh
diff --git a/builtin/apply.c b/builtin/apply.c
index 84a8a0b..78e25fa 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -627,9 +627,13 @@ static char *find_name_common(const char *line, char *def, int p_value,
if (name_terminate(start, line-start, c, terminate))
break;
}
- line++;
- if (c == '/' && !--p_value)
+ if (c == '/' && !--p_value) {
+ while (*line == '/')
+ line++;
start = line;
+ } else {
+ line++;
+ }
}
if (!start)
return squash_slash(def);
diff --git a/t/t4153-apply-doubleslash.sh b/t/t4153-apply-doubleslash.sh
new file mode 100755
index 0000000..1ea76b5
--- /dev/null
+++ b/t/t4153-apply-doubleslash.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+#
+# Copyright (c) 2011 Canonical Ltd.
+#
+
+test_description='git apply filenames with double slashes
+
+Try to apply a patch with git-apply where the filename specified has a double
+slash after the first (to-be-stripped) component'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ test_commit 1 f &&
+ cat > good.patch <<EOF
+diff -u a//f b//f
+--- a//f
++++ b//f
+@@ -1 +1 @@
+-1
++2
+EOF
+'
+
+test_expect_success 'apply diff with double slashes in filenames' '
+ git apply good.patch 2>err
+'
+
+test_done
--
1.7.5.4
^ permalink raw reply related
* Re: Finding info about git svn
From: Sverre Rabbelier @ 2011-11-16 12:39 UTC (permalink / raw)
To: git; +Cc: John 'Warthog9' Hawley, Tim van Heugten, Petr Baudis
In-Reply-To: <CAEtpghyV+w_9hN-hF_Z2MbKAUKyen6e=E9-XSyPxawAyamy3oA@mail.gmail.com>
Heya,
[+git, without HTML part this time, and trying Petr's other address]
On Wed, Nov 16, 2011 at 13:32, Tim van Heugten <stimme@gmail.com> wrote:
> Hi,
>
> When I was looking for info on git svn, the first google hit took me to
> http://git.or.cz/course/svn.html.
> The first thing on the page I notice is a message that the page is no longer
> maintained.
> Unfortunately, GitCrashCourse link from http://git.or.cz/course/svn.html
> gives a 404.
>
> Instead I moved to the root at https://git.wiki.kernel.org to find out more
> about git svn.
> In my hurry I just wanted to jump to the relevant page, and started a
> search.
> Unfortunately, performing a search on git.wiki.kernel.org gives a 404.
>
> I found the relevant page the 'slow' way ;), but I suggest you have a look
> at the search.
> Btw. via the slow way, on the Documentation page I find this: "Git SVN crash
> course (superseded by GitSvnCrashCourse)" - a link back to git.or.cz.
> Shouldn't this link be removed, since it is superseded?
>
> Thanks!
>
>
> Tim van Heugten
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 00/28] Store references hierarchically in cache -- benchmark results
From: Michael Haggerty @ 2011-11-16 12:51 UTC (permalink / raw)
To: mhagger
Cc: Junio C Hamano, git, Jeff King, Drew Northup, Jakub Narebski,
Heiko Voigt, Johan Herland, Julian Phillips
In-Reply-To: <1319804921-17545-1-git-send-email-mhagger@alum.mit.edu>
On 10/28/2011 02:28 PM, mhagger@alum.mit.edu wrote:
> [...]
> This patch series changes how references are stored in the reference
> cache data structures (entirely internal to refs.c). Previously, the
> packed refs were stored in one big array of pointers-to-struct, and
> the loose refs were stored in another. [...]
>
> Therefore, this patch series changes the data structure used to store
> the reference cache from a single array of pointers-to-struct into a
> tree-like structure in which each subdirectory of the reference
> namespace is stored as an array of pointers-to-entry and entries can
> be either references or subdirectories containing more references.
> Moreover, each subdirectory of loose references is only read from disk
> when a reference from that subdirectory (or one of its descendants) is
> needed. This slightly slows down commands that need to iterate
> through all references (simply because the new data structures are
> more complicated), but it *dramatically* decreases the time needed for
> some common operations. For example, in a test repository with 20000
> revisions and 10000 loose tags:
Due to the death of my old computer, I got distracted and never
published the benchmark results that I cited above. (The numbers here
are different than the originals because they are done on a different
computer.) The benchmarks are done using code from [1]; the test
repository consists of a linear series of 20000 commits (with very
little content) and 10000 tags (on every second commit); the tags are
unsharded. The numbers are times in seconds; "cold" means that the disk
cache was flushed immediately before the test; "warm" usually means that
the same operation was done a second time.
Column #0 is Git 1.7.7.2; column #1 is Git 1.7.8-rc2; column #2 is a
recent git master plus my ref-related patch series (what Junio calls
mh/ref-api-take-2 plus the fix that I posted yesterday).
The main change between 1.7.7.2 and 1.7.8 is that the latter stores the
reference cache in a sorted array rather than a linked list, meaning
that it is possible to use bisection to locate a reference quickly by
name rather than having to search linearly through a linked list. This
greatly helps some operations when most references are packed and can be
read from disk quickly. It doesn't make much difference when most
references are loose, because the time required to read the loose
references from disk overwhelms the time for iterating through the array
in memory.
The main improvements between 1.7.8 and the new version are when the old
code would have read all of the loose references but the new code only
needs to read a couple of directories of them. Given that almost all of
the references in the test repository are tags, they often don't need to
be read at all when branches are being manipulated. By contrast, many
old code paths force *all* of the references to be read, for example
when they check for replacements in refs/replace/.
I've done other benchmarks with varying numbers of references. The
results suggest that many operations go from O(N) in the number of loose
references to O(1) (e.g., if all they do is check refs/replace/) or some
other slow scaling that depends on how the reference namespace is organized.
Anything involving packed references necessarily scales at least like
O(N) because packed references are all read at once. OTOH reading
packed refs is so much faster than reading loose references that with
10000 references, packing is still advantageous. (For some number of
references, of course, the curves must cross and loose references will
be more efficient than packed references for some operations.)
The case of git-filter-branch is particularly dramatic; the old code
scales like O(N^2) whereas the new code scales like O(N) as expected.
Moreover, git-filter-branch creates lots of loose references while it
works, so it is not possible to evade the problem by packing the
references before invoking git-filter-branch. I believe that
git-filter-branch is mostly slowed down be each subcommand's check for
replacements in refs/replace (even though there are none in my test
repository) because in the old code this check forces *all* loose
references to be read. Versions 1.7.7 and 1.7.8 of git-filter-branch
runs much faster if the --no-replace-objects option is used.
With these changes, it becomes thinkable to work with repositories with
very large numbers of references (especially if the reference namespace
is sharded appropriately), whereas in 1.7.7 some operations were
annoyingly slow.
Michael
[1] branch "refperf" at git://github.com/mhagger/git.git
> =================================== ======== ======== ========
> Test name [0] [1] [2]
> =================================== ======== ======== ========
> branch-loose-cold 1.59 1.68 0.29
> branch-loose-warm 0.04 0.04 0.00
> for-each-ref-loose-cold 1.86 1.96 1.88
> for-each-ref-loose-warm 0.10 0.10 0.11
> checkout-loose-cold 1.66 1.86 0.39
> checkout-loose-warm 0.04 0.05 0.01
> checkout-orphan-loose 0.04 0.04 0.00
> checkout-from-detached-loose-cold 2.04 2.11 1.81
> checkout-from-detached-loose-warm 0.24 0.15 0.16
> branch-contains-loose-cold 1.79 1.86 1.87
> branch-contains-loose-warm 0.14 0.14 0.14
> pack-refs-loose 0.49 0.53 0.53
> branch-packed-cold 0.28 0.25 0.25
> branch-packed-warm 0.02 0.00 0.00
> for-each-ref-packed-cold 0.34 0.39 0.40
> for-each-ref-packed-warm 0.07 0.07 0.07
> checkout-packed-cold 2.81 0.50 0.55
> checkout-packed-warm 0.01 0.00 0.01
> checkout-orphan-packed 0.00 0.00 0.00
> checkout-from-detached-packed-cold 2.83 0.55 0.46
> checkout-from-detached-packed-warm 2.45 0.12 0.13
> branch-contains-packed-cold 0.38 0.32 0.43
> branch-contains-packed-warm 0.11 0.11 0.11
> clone-loose-cold 30.16 30.31 30.51
> clone-loose-warm 1.28 1.30 1.33
> fetch-nothing-loose 0.21 0.39 0.38
> pack-refs 0.14 0.12 0.14
> fetch-nothing-packed 0.21 0.40 0.39
> clone-packed-cold 1.07 1.24 1.18
> clone-packed-warm 0.23 0.23 0.22
> fetch-everything-cold 30.49 30.89 31.09
> fetch-everything-warm 1.78 2.01 2.06
> filter-branch-warm 2949.81 2891.51 440.60
> =================================== ======== ======== ========
>
>
> [0] 8d19b44 (tag: v1.7.7.2) Git 1.7.7.2
> Test repository created using: t/make-refperf-repo --commits 20000 --refs 10000
> [1] bc1bbe0 (tag: v1.7.8-rc2) Git 1.7.8-rc2
> Test repository created using: t/make-refperf-repo --commits 20000 --refs 10000
> [2] 01494b4 (github/ref-api-D) repack_without_ref(): call clear_packed_ref_cache()
> Test repository created using: t/make-refperf-repo --commits 20000 --refs 10000
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] git-show-ref: fix escaping in asciidoc source
From: Thomas Rast @ 2011-11-16 13:08 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <4EC3A45F.9080005@alum.mit.edu>
Michael Haggerty wrote:
> 36 [2]. How can I escape AsciiDoc markup?
>
> Most AsciiDoc inline elements can be suppressed by preceding them with a
> backslash character. These elements include: [...] But there are
> exceptions — see the next question.
[...]
> Note Escaping is unnecessary inside inline literal passthroughs
> (backtick quoted text).
Beware! We actively change the inline literal quoting behaviour to
ensure consistent output with different versions of asciidoc. See
this commit:
commit 71c020c53ec472b04678237d8fe5687f2299db2a
Author: Thomas Rast <trast@student.ethz.ch>
Date: Sat Jul 25 14:06:50 2009 +0200
Disable asciidoc 8.4.1+ semantics for `{plus}` and friends
asciidoc 8.4.1 changed the semantics of inline backtick quoting so
that they disable parsing of inline constructs, i.e.,
Input: `{plus}`
Pre 8.4.1: +
Post 8.4.1: {plus}
Fix this by defining the asciidoc attribute 'no-inline-literal'
(which, per the 8.4.1 changelog, is the toggle to return to the old
behaviour) when under ASCIIDOC8.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* feature request: git format-patch - allow --suffix to work with --numbered-files
From: MikeW @ 2011-11-16 13:02 UTC (permalink / raw)
To: git
"Numbered files" is good for creating patches with uniform filenames, rather
than verbose filenames containing part of the commit message, but if working for
example with submodules, there is possible name collision if pooling the patches
in one place.
I thought that --suffix=$name would be a good way to disambiguate but found that
--numbered-files wins and kills any suffix.
Hence suggestion that --suffix used with --numbered-files is allowed to generate
a filename suffix as it does when not in "numbered files" mode.
In the absence of a --suffix command, the behaviour would be as now.
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Nguyen Thai Ngoc Duy @ 2011-11-16 13:33 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <20111116075955.GB13706@elie.hsd1.il.comcast.net>
2011/11/16 Jonathan Nieder <jrnieder@gmail.com>:
> Jokes aside, here's a rough series to do the git_path ->
> git_path_unsafe renaming. While writing it, I noticed a couple of
> bugs, hence the two patches before the last one. Patch 2 is the more
> interesting one.
Another approach is do nothing and leave it for a static analysis tool
to detect potential problems. I'm looking at sparse at the moment,
although I know nothing about it to say if it can or cannot detect
such problems. We can at least make sparse detect return value from
git_path() being passed to an unsafe function, I think.
--
Duy
^ permalink raw reply
* Re: Merging back from master but keeping a subtree
From: Steinar Bang @ 2011-11-16 13:39 UTC (permalink / raw)
To: git
In-Reply-To: <20110918033719.GB17977@sigill.intra.peff.net>
>>>>> Jeff King <peff@peff.net>:
> Git should generally do that automatically, unless both sides are
> changing mydirectory. In which case it will produce conflicts.
I thought so too, but the end result didn't build, and I was unable to
figure out why.
> Are you sure you really want to just throw out what the other side did
> in mydirectory?
Yes, that's the only thing I'm sure about. All changes to mydirectory
should come from my branch. If there are changes necessary to make
things build, they are better done by me.
> If git was able to auto-merge some files, then they will not be marked
> as conflicts in the index. And "git checkout --ours" is about looking in
> the index for conflicted entries, and then selecting one side.
> I think what you want instead is to do is (assuming you really want to
> throw out their side):
Thanks for the suggestion. I tried following this approach, but...
> 1. Start a merge between them and us:
> git merge --no-commit remoterepo/master
> 2. Throw out whatever the merge came up with and make it look like
> their tree:
...I never quite could figure out if I did the right thing here.
Ie. when throwing out what the commit came up with.
> git checkout remoterepo/master -- top
> 3. Now overwrite their version of mydirectory with what was in your
> branch:
> git checkout HEAD -- top/middle/mydirectory
> 4. Commit the resulting tree:
> git commit
The problem was that the end result didn't build, wit pretty much the
same obscure failures that the regular merge had.
But I eventually tried something that worked:
1. First create a local branch off master
2. Merge in mybranch with "-s ours --no-commit"
3. Checkout mydirectory from mybranch
4. Commit
5. Switch to mybranch
6. Merge in the new local branch, which resulted in a conflict-free
merge that has my changes in mydirectory and the rest of the world
in the rest of the working directory, and the history of mydirectory
looks ok
The end result built and worked.
If git had had a "-s theirs" strategy, I wouldn't have needed the
temporary branch made off master. That need for that branch is probably
the most cryptic thing for people following my instructions for future
merges.
To sum up the commands:
git checkout master
git checkout -b master_nicely_merged_with_mybranch
git merge -s ours --no-commit origin/mybranch
git checkout origin/mybranch top/middle/mydirectory
git commit
git checkout mybranch
git merge master_nicely_merged_with_mybranch
Thanks!
- Steinar
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Michael Haggerty @ 2011-11-16 13:44 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jonathan Nieder, Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <CACsJy8A2=qBiyY3SD-PZo+E=U+Dfjm1UQidgq6khQARZ3d41WQ@mail.gmail.com>
On 11/16/2011 02:33 PM, Nguyen Thai Ngoc Duy wrote:
> 2011/11/16 Jonathan Nieder <jrnieder@gmail.com>:
>> Jokes aside, here's a rough series to do the git_path ->
>> git_path_unsafe renaming. While writing it, I noticed a couple of
>> bugs, hence the two patches before the last one. Patch 2 is the more
>> interesting one.
>
> Another approach is do nothing and leave it for a static analysis tool
> to detect potential problems. I'm looking at sparse at the moment,
> although I know nothing about it to say if it can or cannot detect
> such problems. We can at least make sparse detect return value from
> git_path() being passed to an unsafe function, I think.
For the cases when static analysis doesn't suffice, recently I posted
some patches that make it possible for debug a problem that results from
the use of a "stale" buffer [1]. But having myself also been bitten by
this problem, I'd also be in favor of a more systematic solution, even
if it has a small runtime cost. After all, most of the time the
filename created by git_path() is going to be passed to the kernel a
moment later, which will usually be vastly slower than an extra malloc/free.
Michael
[1] http://comments.gmane.org/gmane.comp.version-control.git/182209
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [RFC 2/2] Make misuse of get_pathname() buffers detectable by valgrind
From: Nguyen Thai Ngoc Duy @ 2011-11-16 13:57 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, Junio C Hamano, Petr Baudis
In-Reply-To: <1317097687-11098-3-git-send-email-mhagger@alum.mit.edu>
On Tue, Sep 27, 2011 at 11:28 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> A temporary buffer produced by get_pathname() is recycled after a few
> subsequent calls of get_pathname(). The use of such a buffer after it
> has been recycled can result in the wrong file being accessed with
> very strange effects. Moreover, such a bug can lie dormant until code
> elsewhere is changed to use a temporary buffer, causing very
> mysterious, nonlocal failures that are hard to analyze.
>
> Add a second implementation of get_pathname() (activated if the
> VALGRIND preprocessor macro is defined) that allocates and frees
> buffers instead of recycling statically-allocated buffers. This does
> not make the problem less serious, but it turns the errors into
> access-after-free errors, making it possible to locate the guilty code
> using valgrind.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>
> I believe that it is frowned upon to use #ifdefs in git code, but no
> good alternative is obvious to me for this type of use. Suggestions
> are welcome.
Enable the code based on an environment variable, e.g.
GIT_DEBUG_FENCE, then enable it by default in test-lib.sh :-)
--
Duy
^ permalink raw reply
* Re: [RFC 2/2] Make misuse of get_pathname() buffers detectable by valgrind
From: Nguyen Thai Ngoc Duy @ 2011-11-16 14:18 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, Junio C Hamano
In-Reply-To: <1317097687-11098-3-git-send-email-mhagger@alum.mit.edu>
On Tue, Sep 27, 2011 at 11:28 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> +#ifdef VALGRIND
> + static char *pathname_array[PATHNAME_BUFFER_COUNT];
> + index = (index + 1) & (PATHNAME_BUFFER_COUNT - 1);
> + if (pathname_array[index]) {
> + /*
> + * In a correct program, this will have no effect, but
> + * *if* somebody erroneously uses this buffer after it
> + * has been freed, it gives more of a chance that the
> + * error will be detected even if valgrind is not
> + * running:
> + */
> + strcpy(pathname_array[index], buggy_path);
> +
> + free(pathname_array[index]);
> + }
> + pathname_array[index] = xmalloc(PATH_MAX);
> + return pathname_array[index];
> +#else
Not sure if it works (just read man pages, haven't tried anything) I'm
thinking to use mmap() with MAP_ANONYMOUS instead of xmalloc(), then
mprotect() instead of free() to remove read access from that area. Any
access after that should be caught. Leaking may not be severe for
git_path(), hopefully.
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox