onsdag 28 november 2007 skrev Junio C Hamano: > This looks somewhat tighter than the previous one, but still made me > worried if the caller of prefix_path() has run the setup sequence enough > so that calling get_git_work_tree() is safe, so I ended up auditing the > callpath. At least, I do not want to see that unconditional call to > get_git_work_tree() when we do not need to do this "ah prefix got an > unusual absolute path" stuff. > > * builtin-init-db.c uses prefix_path() to find where the template is > (this is mingw fallout change); in general, I do not think we would > want to trigger repository nor worktree discovery inside init-db, > although I suspect this particular callpath could be made Ok (because > it is taken only when template_dir is not absolute) if you do not > unconditionally call get_git_work_tree() in prefix_path(). > > * config.c uses prefix_path() to find the ETC_GITCONFIG that is not > absolute (again, mingw fallout). When git_config() is called, we > already should have discovered repository but worktree may not have > been found yet (config.worktree can be used to specify where it is, > so you have a chicken and egg problem). Again, this particular > callpath happens to be Ok because this is used only for non-absolute > path, but that is a bit subtle. I wonder if this usage in config and initdb is what prefix_path() was intended for. The interface is declared in cache.h and there are error conditions like '%s is outside repository'. Maybe we should have a boolean indicating whether the arguments refer to filespecs or not to make this clear or rewite the mingw fallouts in some other way. > * get_pathspec() uses prefix_path() for obvious reasons, and the prefix > it gets must have been discovered by finding out where the worktree > is, so by definition that one is safe. > > Everybody else you would get from "git grep prefix_path" are after the > proper setup, so they should all be safe. Thanks for looking at the usages. I'll come up with some more tests too, though writing negative tests sometimes is a challenge. Tests all to easily fail for the wrong reason which is bad when we expect them to fail for the right reason. -- robin - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
