* [PATCH v3 02/22] git check-ref-format: add options --allow-onelevel and --refspec-pattern
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Also add tests of the new options. (Actually, one big reason to add
the new options is to make it easy to test check_ref_format(), though
the options should also be useful to other scripts.)
Interpret the result of check_ref_format() based on which types of
refnames are allowed. However, because check_ref_format() can only
return a single value, one test case is still broken. Specifically,
the case "git check-ref-format --onelevel '*'" incorrectly succeeds
because check_ref_format() returns CHECK_REF_FORMAT_ONELEVEL for this
refname even though the refname is also CHECK_REF_FORMAT_WILDCARD.
The type of check that leads to this failure is used elsewhere in
"real" code and could lead to bugs; it will be fixed over the next few
commits.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 29 +++++++++--
builtin/check-ref-format.c | 56 +++++++++++++++++---
t/t1402-check-ref-format.sh | 88 +++++++++++++++++++++++++++++---
3 files changed, 152 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index c9fdf84..dcb8cc3 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -8,8 +8,8 @@ git-check-ref-format - Ensures that a reference name is well formed
SYNOPSIS
--------
[verse]
-'git check-ref-format' <refname>
-'git check-ref-format' --print <refname>
+'git check-ref-format' [--print]
+ [--[no-]allow-onelevel] [--refspec-pattern] <refname>
'git check-ref-format' --branch <branchname-shorthand>
DESCRIPTION
@@ -32,14 +32,18 @@ git imposes the following rules on how references are named:
. They must contain at least one `/`. This enforces the presence of a
category like `heads/`, `tags/` etc. but the actual names are not
- restricted.
+ restricted. If the `--allow-onelevel` option is used, this rule
+ is waived.
. They cannot have two consecutive dots `..` anywhere.
. They cannot have ASCII control characters (i.e. bytes whose
values are lower than \040, or \177 `DEL`), space, tilde `~`,
- caret `{caret}`, colon `:`, question-mark `?`, asterisk `*`,
- or open bracket `[` anywhere.
+ caret `{caret}`, or colon `:` anywhere.
+
+. They cannot have question-mark `?`, asterisk `{asterisk}`, or open
+ bracket `[` anywhere. See the `--refspec-pattern` option below for
+ an exception to this rule.
. They cannot end with a slash `/` nor a dot `.`.
@@ -78,6 +82,21 @@ were on. This option should be used by porcelains to accept this
syntax anywhere a branch name is expected, so they can act as if you
typed the branch name.
+OPTIONS
+-------
+--allow-onelevel::
+--no-allow-onelevel::
+ Controls whether one-level refnames are accepted (i.e.,
+ refnames that do not contain multiple `/`-separated
+ components). The default is `--no-allow-onelevel`.
+
+--refspec-pattern::
+ Interpret <refname> as a reference name pattern for a refspec
+ (as used with remote repositories). If this option is
+ enabled, <refname> is allowed to contain a single `{asterisk}`
+ in place of a one full pathname component (e.g.,
+ `foo/{asterisk}/bar` but not `foo/bar{asterisk}`).
+
EXAMPLES
--------
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 0723cf2..7295954 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -8,7 +8,7 @@
#include "strbuf.h"
static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--print] <refname>\n"
+"git check-ref-format [--print] [options] <refname>\n"
" or: git check-ref-format --branch <branchname-shorthand>";
/*
@@ -45,27 +45,65 @@ static int check_ref_format_branch(const char *arg)
return 0;
}
-static int check_ref_format_print(const char *arg)
+static void refname_format_print(const char *arg)
{
char *refname = xmalloc(strlen(arg) + 1);
- if (check_ref_format(arg))
- return 1;
collapse_slashes(refname, arg);
printf("%s\n", refname);
- return 0;
}
+#define REFNAME_ALLOW_ONELEVEL 1
+#define REFNAME_REFSPEC_PATTERN 2
+
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
+ int i;
+ int print = 0;
+ int flags = 0;
+
if (argc == 2 && !strcmp(argv[1], "-h"))
usage(builtin_check_ref_format_usage);
if (argc == 3 && !strcmp(argv[1], "--branch"))
return check_ref_format_branch(argv[2]);
- if (argc == 3 && !strcmp(argv[1], "--print"))
- return check_ref_format_print(argv[2]);
- if (argc != 2)
+
+ for (i = 1; i < argc && argv[i][0] == '-'; i++) {
+ if (!strcmp(argv[i], "--print"))
+ print = 1;
+ else if (!strcmp(argv[i], "--allow-onelevel"))
+ flags |= REFNAME_ALLOW_ONELEVEL;
+ else if (!strcmp(argv[i], "--no-allow-onelevel"))
+ flags &= ~REFNAME_ALLOW_ONELEVEL;
+ else if (!strcmp(argv[i], "--refspec-pattern"))
+ flags |= REFNAME_REFSPEC_PATTERN;
+ else
+ usage(builtin_check_ref_format_usage);
+ }
+ if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- return !!check_ref_format(argv[1]);
+
+ switch (check_ref_format(argv[i])) {
+ case CHECK_REF_FORMAT_OK:
+ break;
+ case CHECK_REF_FORMAT_ERROR:
+ return 1;
+ case CHECK_REF_FORMAT_ONELEVEL:
+ if (!(flags & REFNAME_ALLOW_ONELEVEL))
+ return 1;
+ else
+ break;
+ case CHECK_REF_FORMAT_WILDCARD:
+ if (!(flags & REFNAME_REFSPEC_PATTERN))
+ return 1;
+ else
+ break;
+ default:
+ die("internal error: unexpected value from check_ref_format()");
+ }
+
+ if (print)
+ refname_format_print(argv[i]);
+
+ return 0;
}
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index dc43171..f551eef 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -5,25 +5,38 @@ test_description='Test git check-ref-format'
. ./test-lib.sh
valid_ref() {
- test_expect_success "ref name '$1' is valid" \
- "git check-ref-format '$1'"
+ if test "$#" = 1
+ then
+ test_expect_success "ref name '$1' is valid" \
+ "git check-ref-format '$1'"
+ else
+ test_expect_success "ref name '$1' is valid with options $2" \
+ "git check-ref-format $2 '$1'"
+ fi
}
invalid_ref() {
- test_expect_success "ref name '$1' is not valid" \
- "test_must_fail git check-ref-format '$1'"
+ if test "$#" = 1
+ then
+ test_expect_success "ref name '$1' is invalid" \
+ "test_must_fail git check-ref-format '$1'"
+ else
+ test_expect_success "ref name '$1' is invalid with options $2" \
+ "test_must_fail git check-ref-format $2 '$1'"
+ fi
}
invalid_ref ''
invalid_ref '/'
-valid_ref 'heads/foo'
-invalid_ref 'foo'
+invalid_ref '/' --allow-onelevel
valid_ref 'foo/bar/baz'
valid_ref 'refs///heads/foo'
invalid_ref 'heads/foo/'
valid_ref '/heads/foo'
valid_ref '///heads/foo'
-invalid_ref '/foo'
invalid_ref './foo'
+invalid_ref './foo/bar'
+invalid_ref 'foo/./bar'
+invalid_ref 'foo/bar/.'
invalid_ref '.refs/foo'
invalid_ref 'heads/foo..bar'
invalid_ref 'heads/foo?bar'
@@ -38,6 +51,67 @@ invalid_ref 'heads/foo\bar'
invalid_ref "$(printf 'heads/foo\t')"
invalid_ref "$(printf 'heads/foo\177')"
valid_ref "$(printf 'heads/fu\303\237')"
+invalid_ref 'heads/*foo/bar' --refspec-pattern
+invalid_ref 'heads/foo*/bar' --refspec-pattern
+invalid_ref 'heads/f*o/bar' --refspec-pattern
+
+ref='foo'
+invalid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/bar'
+valid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/*'
+invalid_ref "$ref"
+invalid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*/foo'
+invalid_ref "$ref"
+invalid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/*/bar'
+invalid_ref "$ref"
+invalid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*'
+invalid_ref "$ref"
+
+#invalid_ref "$ref" --allow-onelevel
+test_expect_failure "ref name '$ref' is invalid with options --allow-onelevel" \
+ "test_must_fail git check-ref-format --allow-onelevel '$ref'"
+
+invalid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/*/*'
+invalid_ref "$ref" --refspec-pattern
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*/foo/*'
+invalid_ref "$ref" --refspec-pattern
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*/*/foo'
+invalid_ref "$ref" --refspec-pattern
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='/foo'
+invalid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 06/22] Do not allow ".lock" at the end of any refname component
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Allowing any refname component to end with ".lock" is looking for
trouble; for example,
$ git br foo.lock/bar
$ git br foo
fatal: Unable to create '[...]/.git/refs/heads/foo.lock': File exists.
Therefore, do not allow any refname component to end with ".lock".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
This change was discussed on the mailing list [1]. It is regrettable
that we can't change the name of the lock files to something that
cannot appear in a reference name (like .refname.lock), but such a
change would cause problems if two versions of git are simultaneously
accessing the same repository.
[1] http://thread.gmane.org/gmane.comp.version-control.git/181051/focus=181069
Documentation/git-check-ref-format.txt | 4 +---
refs.c | 4 ++--
t/t1402-check-ref-format.sh | 8 ++++----
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index dcb8cc3..9114751 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -28,7 +28,7 @@ git imposes the following rules on how references are named:
. They can include slash `/` for hierarchical (directory)
grouping, but no slash-separated component can begin with a
- dot `.`.
+ dot `.` or end with the sequence `.lock`.
. They must contain at least one `/`. This enforces the presence of a
category like `heads/`, `tags/` etc. but the actual names are not
@@ -47,8 +47,6 @@ git imposes the following rules on how references are named:
. They cannot end with a slash `/` nor a dot `.`.
-. They cannot end with the sequence `.lock`.
-
. They cannot contain a sequence `@{`.
. They cannot contain a `\`.
diff --git a/refs.c b/refs.c
index 5259724..5a0bd0f 100644
--- a/refs.c
+++ b/refs.c
@@ -898,6 +898,8 @@ static int check_refname_component(const char *ref)
return -1; /* Component has zero length. */
if (ref[0] == '.')
return -1; /* Component starts with '.'. */
+ if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
+ return -1; /* Refname ends with ".lock". */
return cp - ref;
}
@@ -931,8 +933,6 @@ int check_refname_format(const char *ref, int flags)
if (ref[component_len - 1] == '.')
return -1; /* Refname ends with '.'. */
- if (component_len >= 5 && !memcmp(&ref[component_len - 5], ".lock", 5))
- return -1; /* Refname ends with ".lock". */
if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
return -1; /* Refname has only one component. */
return 0;
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1cad88f..419788f 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -43,8 +43,8 @@ invalid_ref 'heads/foo?bar'
valid_ref 'foo./bar'
invalid_ref 'heads/foo.lock'
invalid_ref 'heads///foo.lock'
-valid_ref 'foo.lock/bar'
-valid_ref 'foo.lock///bar'
+invalid_ref 'foo.lock/bar'
+invalid_ref 'foo.lock///bar'
valid_ref 'heads/foo@bar'
invalid_ref 'heads/v@{ation'
invalid_ref 'heads/foo\bar'
@@ -160,7 +160,7 @@ invalid_ref_normalized 'heads/./foo'
invalid_ref_normalized 'heads\foo'
invalid_ref_normalized 'heads/foo.lock'
invalid_ref_normalized 'heads///foo.lock'
-valid_ref_normalized 'foo.lock/bar' 'foo.lock/bar'
-valid_ref_normalized 'foo.lock///bar' 'foo.lock/bar'
+invalid_ref_normalized 'foo.lock/bar'
+invalid_ref_normalized 'foo.lock///bar'
test_done
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 08/22] Inline function refname_format_print()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Soon we will make printing independent of collapsing.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 989ee5c..f5df9aa 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -48,17 +48,12 @@ static int check_ref_format_branch(const char *arg)
return 0;
}
-static void refname_format_print(const char *arg)
-{
- char *refname = collapse_slashes(arg);
- printf("%s\n", refname);
-}
-
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
int print = 0;
int flags = 0;
+ const char *refname;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage(builtin_check_ref_format_usage);
@@ -81,11 +76,14 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- if (check_refname_format(argv[i], flags))
+ refname = argv[i];
+ if (check_refname_format(refname, flags))
return 1;
- if (print)
- refname_format_print(argv[i]);
+ if (print) {
+ refname = collapse_slashes(refname);
+ printf("%s\n", refname);
+ }
return 0;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 09/22] Change check_refname_format() to reject unnormalized refnames
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Since much of the infrastructure does not work correctly with
unnormalized refnames, change check_refname_format() to reject them.
Similarly, change "git check-ref-format" to reject unnormalized
refnames by default. But add an option --normalize, which causes "git
check-ref-format" to normalize the refname before checking its format,
and print the normalized refname. This is exactly the behavior of the
old --print option, which is retained but deprecated.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 26 ++++++++++++++++++--------
builtin/check-ref-format.c | 15 +++++++--------
refs.c | 3 ---
refs.h | 2 +-
t/t1402-check-ref-format.sh | 31 +++++++++++++++++++++++--------
5 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index 9114751..103e7b1 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -8,8 +8,9 @@ git-check-ref-format - Ensures that a reference name is well formed
SYNOPSIS
--------
[verse]
-'git check-ref-format' [--print]
- [--[no-]allow-onelevel] [--refspec-pattern] <refname>
+'git check-ref-format' [--normalize]
+ [--[no-]allow-onelevel] [--refspec-pattern]
+ <refname>
'git check-ref-format' --branch <branchname-shorthand>
DESCRIPTION
@@ -45,7 +46,11 @@ git imposes the following rules on how references are named:
bracket `[` anywhere. See the `--refspec-pattern` option below for
an exception to this rule.
-. They cannot end with a slash `/` nor a dot `.`.
+. They cannot begin or end with a slash `/` or contain multiple
+ consecutive slashes (see the `--normalize` option below for an
+ exception to this rule)
+
+. They cannot end with a dot `.`.
. They cannot contain a sequence `@{`.
@@ -70,10 +75,6 @@ reference name expressions (see linkgit:gitrevisions[7]):
. at-open-brace `@{` is used as a notation to access a reflog entry.
-With the `--print` option, if 'refname' is acceptable, it prints the
-canonicalized name of a hypothetical reference with that name. That is,
-it prints 'refname' with any extra `/` characters removed.
-
With the `--branch` option, it expands the ``previous branch syntax''
`@{-n}`. For example, `@{-1}` is a way to refer the last branch you
were on. This option should be used by porcelains to accept this
@@ -95,6 +96,15 @@ OPTIONS
in place of a one full pathname component (e.g.,
`foo/{asterisk}/bar` but not `foo/bar{asterisk}`).
+--normalize::
+ Normalize 'refname' by removing any leading slash (`/`)
+ characters and collapsing runs of adjacent slashes between
+ name components into a single slash. Iff the normalized
+ refname is valid then print it to standard output and exit
+ with a status of 0. (`--print` is a deprecated way to spell
+ `--normalize`.)
+
+
EXAMPLES
--------
@@ -107,7 +117,7 @@ $ git check-ref-format --branch @{-1}
* Determine the reference name to use for a new branch:
+
------------
-$ ref=$(git check-ref-format --print "refs/heads/$newbranch") ||
+$ ref=$(git check-ref-format --normalize "refs/heads/$newbranch") ||
die "we do not like '$newbranch' as a branch name."
------------
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index f5df9aa..28a7320 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -8,7 +8,7 @@
#include "strbuf.h"
static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--print] [options] <refname>\n"
+"git check-ref-format [--normalize] [options] <refname>\n"
" or: git check-ref-format --branch <branchname-shorthand>";
/*
@@ -51,7 +51,7 @@ static int check_ref_format_branch(const char *arg)
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
- int print = 0;
+ int normalize = 0;
int flags = 0;
const char *refname;
@@ -62,8 +62,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
return check_ref_format_branch(argv[2]);
for (i = 1; i < argc && argv[i][0] == '-'; i++) {
- if (!strcmp(argv[i], "--print"))
- print = 1;
+ if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
+ normalize = 1;
else if (!strcmp(argv[i], "--allow-onelevel"))
flags |= REFNAME_ALLOW_ONELEVEL;
else if (!strcmp(argv[i], "--no-allow-onelevel"))
@@ -77,13 +77,12 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
usage(builtin_check_ref_format_usage);
refname = argv[i];
+ if (normalize)
+ refname = collapse_slashes(refname);
if (check_refname_format(refname, flags))
return 1;
-
- if (print) {
- refname = collapse_slashes(refname);
+ if (normalize)
printf("%s\n", refname);
- }
return 0;
}
diff --git a/refs.c b/refs.c
index 5a0bd0f..d2aac24 100644
--- a/refs.c
+++ b/refs.c
@@ -908,9 +908,6 @@ int check_refname_format(const char *ref, int flags)
int component_len, component_count = 0;
while (1) {
- while (*ref == '/')
- ref++; /* tolerate leading and repeated slashes */
-
/* We are at the start of a path component. */
component_len = check_refname_component(ref);
if (component_len < 0) {
diff --git a/refs.h b/refs.h
index 48540c0..b0da5fc 100644
--- a/refs.h
+++ b/refs.h
@@ -106,7 +106,7 @@ extern int for_each_reflog(each_ref_fn, void *);
* REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
* reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
* allow a "*" wildcard character in place of one of the name
- * components.
+ * components. No leading or repeated slashes are accepted.
*/
extern int check_refname_format(const char *ref, int flags);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 419788f..710fcca 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -28,11 +28,17 @@ invalid_ref() {
invalid_ref ''
invalid_ref '/'
invalid_ref '/' --allow-onelevel
+invalid_ref '/' --normalize
+invalid_ref '/' '--allow-onelevel --normalize'
valid_ref 'foo/bar/baz'
-valid_ref 'refs///heads/foo'
+valid_ref 'foo/bar/baz' --normalize
+invalid_ref 'refs///heads/foo'
+valid_ref 'refs///heads/foo' --normalize
invalid_ref 'heads/foo/'
-valid_ref '/heads/foo'
-valid_ref '///heads/foo'
+invalid_ref '/heads/foo'
+valid_ref '/heads/foo' --normalize
+invalid_ref '///heads/foo'
+valid_ref '///heads/foo' --normalize
invalid_ref './foo'
invalid_ref './foo/bar'
invalid_ref 'foo/./bar'
@@ -60,12 +66,15 @@ invalid_ref "$ref"
valid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
ref='foo/bar'
valid_ref "$ref"
valid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+valid_ref "$ref" --normalize
ref='foo/*'
invalid_ref "$ref"
@@ -78,6 +87,8 @@ invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--refspec-pattern --normalize'
ref='foo/*/bar'
invalid_ref "$ref"
@@ -105,9 +116,13 @@ invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
ref='/foo'
invalid_ref "$ref"
-valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
-valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
+invalid_ref "$ref" '--refspec-pattern --normalize'
+valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
@@ -141,12 +156,12 @@ test_expect_success 'check-ref-format --branch from subdir' '
valid_ref_normalized() {
test_expect_success "ref name '$1' simplifies to '$2'" "
- refname=\$(git check-ref-format --print '$1') &&
+ refname=\$(git check-ref-format --normalize '$1') &&
test \"\$refname\" = '$2'"
}
invalid_ref_normalized() {
- test_expect_success "check-ref-format --print rejects '$1'" "
- test_must_fail git check-ref-format --print '$1'"
+ test_expect_success "check-ref-format --normalize rejects '$1'" "
+ test_must_fail git check-ref-format --normalize '$1'"
}
valid_ref_normalized 'heads/foo' 'heads/foo'
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 07/22] Make collapse_slashes() allocate memory for its result
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
This will make upcoming changes a tiny bit easier.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 8f41696..989ee5c 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -12,25 +12,28 @@ static const char builtin_check_ref_format_usage[] =
" or: git check-ref-format --branch <branchname-shorthand>";
/*
- * Remove leading slashes and replace each run of adjacent slashes in
- * src with a single slash, and write the result to dst.
+ * Return a copy of refname but with leading slashes removed and runs
+ * of adjacent slashes replaced with single slashes.
*
* This function is similar to normalize_path_copy(), but stripped down
* to meet check_ref_format's simpler needs.
*/
-static void collapse_slashes(char *dst, const char *src)
+static char *collapse_slashes(const char *refname)
{
+ char *ret = xmalloc(strlen(refname) + 1);
char ch;
char prev = '/';
+ char *cp = ret;
- while ((ch = *src++) != '\0') {
+ while ((ch = *refname++) != '\0') {
if (prev == '/' && ch == prev)
continue;
- *dst++ = ch;
+ *cp++ = ch;
prev = ch;
}
- *dst = '\0';
+ *cp = '\0';
+ return ret;
}
static int check_ref_format_branch(const char *arg)
@@ -47,9 +50,7 @@ static int check_ref_format_branch(const char *arg)
static void refname_format_print(const char *arg)
{
- char *refname = xmalloc(strlen(arg) + 1);
-
- collapse_slashes(refname, arg);
+ char *refname = collapse_slashes(arg);
printf("%s\n", refname);
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 05/22] Refactor check_refname_format()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Among other things, extract a function check_refname_component().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 95 +++++++++++++++++++++++++++++++++++++---------------------------
1 files changed, 55 insertions(+), 40 deletions(-)
diff --git a/refs.c b/refs.c
index aaa8730..5259724 100644
--- a/refs.c
+++ b/refs.c
@@ -872,55 +872,70 @@ static inline int bad_ref_char(int ch)
return 0;
}
+/*
+ * Try to read one refname component from the front of ref. Return
+ * the length of the component found, or -1 if the component is not
+ * legal.
+ */
+static int check_refname_component(const char *ref)
+{
+ const char *cp;
+ char last = '\0';
+
+ for (cp = ref; ; cp++) {
+ char ch = *cp;
+ if (ch == '\0' || ch == '/')
+ break;
+ if (bad_ref_char(ch))
+ return -1; /* Illegal character in refname. */
+ if (last == '.' && ch == '.')
+ return -1; /* Refname contains "..". */
+ if (last == '@' && ch == '{')
+ return -1; /* Refname contains "@{". */
+ last = ch;
+ }
+ if (cp == ref)
+ return -1; /* Component has zero length. */
+ if (ref[0] == '.')
+ return -1; /* Component starts with '.'. */
+ return cp - ref;
+}
+
int check_refname_format(const char *ref, int flags)
{
- int ch, level, last;
- const char *cp = ref;
+ int component_len, component_count = 0;
- level = 0;
while (1) {
- while ((ch = *cp++) == '/')
- ; /* tolerate duplicated slashes */
- if (!ch)
- /* should not end with slashes */
- return -1;
-
- /* we are at the beginning of the path component */
- if (ch == '.')
- return -1;
- if (bad_ref_char(ch)) {
- if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
- (!*cp || *cp == '/'))
+ while (*ref == '/')
+ ref++; /* tolerate leading and repeated slashes */
+
+ /* We are at the start of a path component. */
+ component_len = check_refname_component(ref);
+ if (component_len < 0) {
+ if ((flags & REFNAME_REFSPEC_PATTERN) &&
+ ref[0] == '*' &&
+ (ref[1] == '\0' || ref[1] == '/')) {
/* Accept one wildcard as a full refname component. */
flags &= ~REFNAME_REFSPEC_PATTERN;
- else
- return -1;
- }
-
- last = ch;
- /* scan the rest of the path component */
- while ((ch = *cp++) != 0) {
- if (bad_ref_char(ch))
- return -1;
- if (ch == '/')
- break;
- if (last == '.' && ch == '.')
- return -1;
- if (last == '@' && ch == '{')
- return -1;
- last = ch;
- }
- level++;
- if (!ch) {
- if (ref <= cp - 2 && cp[-2] == '.')
- return -1;
- if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
- return -1;
- if (has_extension(ref, ".lock"))
+ component_len = 1;
+ } else {
return -1;
- return 0;
+ }
}
+ component_count++;
+ if (ref[component_len] == '\0')
+ break;
+ /* Skip to next component. */
+ ref += component_len + 1;
}
+
+ if (ref[component_len - 1] == '.')
+ return -1; /* Refname ends with '.'. */
+ if (component_len >= 5 && !memcmp(&ref[component_len - 5], ".lock", 5))
+ return -1; /* Refname ends with ".lock". */
+ if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
+ return -1; /* Refname has only one component. */
+ return 0;
}
const char *prettify_refname(const char *name)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 01/22] t1402: add some more tests
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
The new tests reflect the status quo. Soon the rule for "*.lock" in
refname components will be tightened up.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
t/t1402-check-ref-format.sh | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index ed4275a..dc43171 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -13,6 +13,8 @@ invalid_ref() {
"test_must_fail git check-ref-format '$1'"
}
+invalid_ref ''
+invalid_ref '/'
valid_ref 'heads/foo'
invalid_ref 'foo'
valid_ref 'foo/bar/baz'
@@ -27,6 +29,9 @@ invalid_ref 'heads/foo..bar'
invalid_ref 'heads/foo?bar'
valid_ref 'foo./bar'
invalid_ref 'heads/foo.lock'
+invalid_ref 'heads///foo.lock'
+valid_ref 'foo.lock/bar'
+valid_ref 'foo.lock///bar'
valid_ref 'heads/foo@bar'
invalid_ref 'heads/v@{ation'
invalid_ref 'heads/foo\bar'
@@ -83,5 +88,9 @@ invalid_ref_normalized '/foo'
invalid_ref_normalized 'heads/foo/../bar'
invalid_ref_normalized 'heads/./foo'
invalid_ref_normalized 'heads\foo'
+invalid_ref_normalized 'heads/foo.lock'
+invalid_ref_normalized 'heads///foo.lock'
+valid_ref_normalized 'foo.lock/bar' 'foo.lock/bar'
+valid_ref_normalized 'foo.lock///bar' 'foo.lock/bar'
test_done
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 04/22] Change check_ref_format() to take a flags argument
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Change check_ref_format() to take a flags argument that indicates what
is acceptable in the reference name (analogous to "git
check-ref-format"'s "--allow-onelevel" and "--refspec-pattern"). This
is more convenient for callers and also fixes a failure in the test
suite (and likely elsewhere in the code) by enabling "onelevel" and
"refspec-pattern" to be allowed independently of each other.
Also rename check_ref_format() to check_refname_format() to make it
obvious that it deals with refnames rather than references themselves.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 21 +---------------
builtin/checkout.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/show-ref.c | 2 +-
builtin/tag.c | 4 +-
connect.c | 2 +-
environment.c | 2 +-
fast-import.c | 7 +----
git_remote_helpers/git/git.py | 2 +-
notes-merge.c | 5 ++-
pack-refs.c | 2 +-
refs.c | 42 ++++++++++++++-----------------
refs.h | 17 +++++++++----
remote.c | 53 +++++++++++------------------------------
sha1_name.c | 4 +-
t/t1402-check-ref-format.sh | 6 +----
transport.c | 16 +++---------
walker.c | 2 +-
20 files changed, 69 insertions(+), 126 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 7295954..8f41696 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -53,9 +53,6 @@ static void refname_format_print(const char *arg)
printf("%s\n", refname);
}
-#define REFNAME_ALLOW_ONELEVEL 1
-#define REFNAME_REFSPEC_PATTERN 2
-
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
@@ -83,24 +80,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- switch (check_ref_format(argv[i])) {
- case CHECK_REF_FORMAT_OK:
- break;
- case CHECK_REF_FORMAT_ERROR:
+ if (check_refname_format(argv[i], flags))
return 1;
- case CHECK_REF_FORMAT_ONELEVEL:
- if (!(flags & REFNAME_ALLOW_ONELEVEL))
- return 1;
- else
- break;
- case CHECK_REF_FORMAT_WILDCARD:
- if (!(flags & REFNAME_REFSPEC_PATTERN))
- return 1;
- else
- break;
- default:
- die("internal error: unexpected value from check_ref_format()");
- }
if (print)
refname_format_print(argv[i]);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3bb6525..574d2b6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,
new->name = arg;
setup_branch_path(new);
- if (check_ref_format(new->path) == CHECK_REF_FORMAT_OK &&
+ if (!check_refname_format(new->path, 0) &&
resolve_ref(new->path, branch_rev, 1, NULL))
hashcpy(rev, branch_rev);
else
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 412bd32..b51e478 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5) &&
- check_ref_format(ref->name + 5))
+ check_refname_format(ref->name + 5, 0))
; /* trash */
else if (args.fetch_all &&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ae164da..0600efa 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -356,7 +356,7 @@ static const char *update(struct command *cmd)
struct ref_lock *lock;
/* only refs/... are allowed */
- if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
+ if (prefixcmp(name, "refs/") || check_refname_format(name + 5, 0)) {
rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
diff --git a/builtin/replace.c b/builtin/replace.c
index fe3a647..517fa10 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
"refs/replace/%s",
sha1_to_hex(object)) > sizeof(ref) - 1)
die("replace ref name too long: %.*s...", 50, ref);
- if (check_ref_format(ref))
+ if (check_refname_format(ref, 0))
die("'%s' is not a valid ref name.", ref);
if (!resolve_ref(ref, prev, 1, NULL))
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 45f0340..fafb6dd 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)
if (strncmp(ref, match, matchlen))
continue;
}
- if (check_ref_format(ref)) {
+ if (check_refname_format(ref, 0)) {
warning("ref '%s' ignored", ref);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..48be745 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -407,12 +407,12 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
{
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_reset(sb);
strbuf_addf(sb, "refs/tags/%s", name);
- return check_ref_format(sb->buf);
+ return check_refname_format(sb->buf, 0);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
diff --git a/connect.c b/connect.c
index ee1d4b4..51990fa 100644
--- a/connect.c
+++ b/connect.c
@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)
len -= 5;
/* REF_NORMAL means that we don't want the magic fake tag refs */
- if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
+ if ((flags & REF_NORMAL) && check_refname_format(name, 0))
return 0;
/* REF_HEADS means that we want regular branch heads */
diff --git a/environment.c b/environment.c
index e96edcf..8174b70 100644
--- a/environment.c
+++ b/environment.c
@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)
if (strcmp((*c)->buf, "/") != 0)
strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
strbuf_list_free(components);
- if (check_ref_format(buf.buf) != CHECK_REF_FORMAT_OK)
+ if (check_refname_format(buf.buf, 0))
die("bad git namespace path \"%s\"", raw_namespace);
strbuf_addch(&buf, '/');
return strbuf_detach(&buf, NULL);
diff --git a/fast-import.c b/fast-import.c
index 742e7da..f9347f5 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -722,13 +722,8 @@ static struct branch *new_branch(const char *name)
if (b)
die("Invalid attempt to create duplicate branch: %s", name);
- switch (check_ref_format(name)) {
- case 0: break; /* its valid */
- case CHECK_REF_FORMAT_ONELEVEL:
- break; /* valid, but too few '/', allow anyway */
- default:
+ if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
die("Branch name doesn't conform to GIT standards: %s", name);
- }
b = pool_calloc(1, sizeof(struct branch));
b->name = pool_strdup(name);
diff --git a/git_remote_helpers/git/git.py b/git_remote_helpers/git/git.py
index a383e6c..007a1bf 100644
--- a/git_remote_helpers/git/git.py
+++ b/git_remote_helpers/git/git.py
@@ -54,7 +54,7 @@ def valid_git_ref (ref_name):
# The following is a reimplementation of the git check-ref-format
# command. The rules were derived from the git check-ref-format(1)
# manual page. This code should be replaced by a call to
- # check_ref_format() in the git library, when such is available.
+ # check_refname_format() in the git library, when such is available.
if ref_name.endswith('/') or \
ref_name.startswith('.') or \
ref_name.count('/.') or \
diff --git a/notes-merge.c b/notes-merge.c
index e1aaf43..3bbcc9d 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -570,7 +570,8 @@ int notes_merge(struct notes_merge_options *o,
/* Dereference o->local_ref into local_sha1 */
if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
die("Failed to resolve local notes ref '%s'", o->local_ref);
- else if (!check_ref_format(o->local_ref) && is_null_sha1(local_sha1))
+ else if (!check_refname_format(o->local_ref, 0) &&
+ is_null_sha1(local_sha1))
local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
else if (!(local = lookup_commit_reference(local_sha1)))
die("Could not parse local commit %s (%s)",
@@ -583,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,
* Failed to get remote_sha1. If o->remote_ref looks like an
* unborn ref, perform the merge using an empty notes tree.
*/
- if (!check_ref_format(o->remote_ref)) {
+ if (!check_refname_format(o->remote_ref, 0)) {
hashclr(remote_sha1);
remote = NULL;
} else {
diff --git a/pack-refs.c b/pack-refs.c
index 1290570..23bbd00 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -72,7 +72,7 @@ static void try_remove_empty_parents(char *name)
for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
while (*p && *p != '/')
p++;
- /* tolerate duplicate slashes; see check_ref_format() */
+ /* tolerate duplicate slashes; see check_refname_format() */
while (*p == '/')
p++;
}
diff --git a/refs.c b/refs.c
index fd29d89..aaa8730 100644
--- a/refs.c
+++ b/refs.c
@@ -872,10 +872,9 @@ static inline int bad_ref_char(int ch)
return 0;
}
-int check_ref_format(const char *ref)
+int check_refname_format(const char *ref, int flags)
{
int ch, level, last;
- int ret = CHECK_REF_FORMAT_OK;
const char *cp = ref;
level = 0;
@@ -884,41 +883,42 @@ int check_ref_format(const char *ref)
; /* tolerate duplicated slashes */
if (!ch)
/* should not end with slashes */
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
/* we are at the beginning of the path component */
if (ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (bad_ref_char(ch)) {
- if (ch == '*' && (!*cp || *cp == '/') &&
- ret == CHECK_REF_FORMAT_OK)
- ret = CHECK_REF_FORMAT_WILDCARD;
+ if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
+ (!*cp || *cp == '/'))
+ /* Accept one wildcard as a full refname component. */
+ flags &= ~REFNAME_REFSPEC_PATTERN;
else
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
}
last = ch;
/* scan the rest of the path component */
while ((ch = *cp++) != 0) {
if (bad_ref_char(ch))
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (ch == '/')
break;
if (last == '.' && ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (last == '@' && ch == '{')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
last = ch;
}
level++;
if (!ch) {
if (ref <= cp - 2 && cp[-2] == '.')
- return CHECK_REF_FORMAT_ERROR;
- if (level < 2)
- return CHECK_REF_FORMAT_ONELEVEL;
+ return -1;
+ if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
+ return -1;
if (has_extension(ref, ".lock"))
- return CHECK_REF_FORMAT_ERROR;
- return ret;
+ return -1;
+ return 0;
}
}
}
@@ -1103,7 +1103,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
{
char refpath[PATH_MAX];
- if (check_ref_format(ref))
+ if (check_refname_format(ref, 0))
return NULL;
strcpy(refpath, mkpath("refs/%s", ref));
return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
@@ -1111,13 +1111,9 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
{
- switch (check_ref_format(ref)) {
- default:
+ if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
return NULL;
- case 0:
- case CHECK_REF_FORMAT_ONELEVEL:
- return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
- }
+ return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
}
static struct lock_file packlock;
diff --git a/refs.h b/refs.h
index dfb086e..48540c0 100644
--- a/refs.h
+++ b/refs.h
@@ -97,11 +97,18 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi
*/
extern int for_each_reflog(each_ref_fn, void *);
-#define CHECK_REF_FORMAT_OK 0
-#define CHECK_REF_FORMAT_ERROR (-1)
-#define CHECK_REF_FORMAT_ONELEVEL (-2)
-#define CHECK_REF_FORMAT_WILDCARD (-3)
-extern int check_ref_format(const char *target);
+#define REFNAME_ALLOW_ONELEVEL 1
+#define REFNAME_REFSPEC_PATTERN 2
+
+/*
+ * Return 0 iff ref has the correct format for a refname according to
+ * the rules described in Documentation/git-check-ref-format.txt. If
+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
+ * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
+ * allow a "*" wildcard character in place of one of the name
+ * components.
+ */
+extern int check_refname_format(const char *ref, int flags);
extern const char *prettify_refname(const char *refname);
extern char *shorten_unambiguous_ref(const char *ref, int strict);
diff --git a/remote.c b/remote.c
index b8ecfa5..6fcf809 100644
--- a/remote.c
+++ b/remote.c
@@ -493,23 +493,6 @@ static void read_config(void)
}
/*
- * We need to make sure the remote-tracking branches are well formed, but a
- * wildcard refspec in "struct refspec" must have a trailing slash. We
- * temporarily drop the trailing '/' while calling check_ref_format(),
- * and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
- * error return is Ok for a wildcard refspec.
- */
-static int verify_refname(char *name, int is_glob)
-{
- int result;
-
- result = check_ref_format(name);
- if (is_glob && result == CHECK_REF_FORMAT_WILDCARD)
- result = CHECK_REF_FORMAT_OK;
- return result;
-}
-
-/*
* This function frees a refspec array.
* Warning: code paths should be checked to ensure that the src
* and dst pointers are always freeable pointers as well
@@ -532,13 +515,13 @@ static void free_refspecs(struct refspec *refspec, int nr_refspec)
static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
{
int i;
- int st;
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
for (i = 0; i < nr_refspec; i++) {
size_t llen;
int is_glob;
const char *lhs, *rhs;
+ int flags;
is_glob = 0;
@@ -576,6 +559,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
rs[i].pattern = is_glob;
rs[i].src = xstrndup(lhs, llen);
+ flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
if (fetch) {
/*
@@ -585,26 +569,20 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
*/
if (!*rs[i].src)
; /* empty is ok */
- else {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_refname_format(rs[i].src, flags))
+ goto invalid;
/*
* RHS
* - missing is ok, and is same as empty.
* - empty is ok; it means not to store.
* - otherwise it must be a valid looking ref.
*/
- if (!rs[i].dst) {
+ if (!rs[i].dst)
; /* ok */
- } else if (!*rs[i].dst) {
+ else if (!*rs[i].dst)
; /* ok */
- } else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_refname_format(rs[i].dst, flags))
+ goto invalid;
} else {
/*
* LHS
@@ -616,8 +594,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
if (!*rs[i].src)
; /* empty is ok */
else if (is_glob) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].src, flags))
goto invalid;
}
else
@@ -630,14 +607,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
* - otherwise it must be a valid looking ref.
*/
if (!rs[i].dst) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].src, flags))
goto invalid;
} else if (!*rs[i].dst) {
goto invalid;
} else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].dst, flags))
goto invalid;
}
}
@@ -1427,8 +1402,8 @@ int get_fetch_map(const struct ref *remote_refs,
for (rmp = &ref_map; *rmp; ) {
if ((*rmp)->peer_ref) {
- int st = check_ref_format((*rmp)->peer_ref->name + 5);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL) {
+ if (check_refname_format((*rmp)->peer_ref->name + 5,
+ REFNAME_ALLOW_ONELEVEL)) {
struct ref *ignore = *rmp;
error("* Ignoring funny ref '%s' locally",
(*rmp)->peer_ref->name);
@@ -1620,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
int len;
/* we already know it starts with refs/ to get here */
- if (check_ref_format(refname + 5))
+ if (check_refname_format(refname + 5, 0))
return 0;
len = strlen(refname) + 1;
diff --git a/sha1_name.c b/sha1_name.c
index ff5992a..143fd97 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -972,9 +972,9 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
{
strbuf_branchname(sb, name);
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
- return check_ref_format(sb->buf);
+ return check_refname_format(sb->buf, 0);
}
/*
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index f551eef..1cad88f 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -87,11 +87,7 @@ valid_ref "$ref" '--refspec-pattern --allow-onelevel'
ref='*'
invalid_ref "$ref"
-
-#invalid_ref "$ref" --allow-onelevel
-test_expect_failure "ref name '$ref' is invalid with options --allow-onelevel" \
- "test_must_fail git check-ref-format --allow-onelevel '$ref'"
-
+invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
diff --git a/transport.c b/transport.c
index fa279d5..feb2ff5 100644
--- a/transport.c
+++ b/transport.c
@@ -754,18 +754,10 @@ void transport_verify_remote_names(int nr_heads, const char **heads)
continue;
remote = remote ? (remote + 1) : local;
- switch (check_ref_format(remote)) {
- case 0: /* ok */
- case CHECK_REF_FORMAT_ONELEVEL:
- /* ok but a single level -- that is fine for
- * a match pattern.
- */
- case CHECK_REF_FORMAT_WILDCARD:
- /* ok but ends with a pattern-match character */
- continue;
- }
- die("remote part of refspec is not a valid name in %s",
- heads[i]);
+ if (check_refname_format(remote,
+ REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))
+ die("remote part of refspec is not a valid name in %s",
+ heads[i]);
}
}
diff --git a/walker.c b/walker.c
index dce7128..be389dc 100644
--- a/walker.c
+++ b/walker.c
@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
{
if (!get_sha1_hex(target, sha1))
return 0;
- if (!check_ref_format(target)) {
+ if (!check_refname_format(target, 0)) {
struct ref *ref = alloc_ref(target);
if (!walker->fetch_ref(walker, ref)) {
hashcpy(sha1, ref->old_sha1);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 03/22] Change bad_ref_char() to return a boolean value
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Previously most bad characters were indicated by returning 1, but "*"
was special-cased to return 2 instead of 1. One caller examined the
return value to see whether the special case occurred.
But it is easier (to document and understand) for bad_ref_char()
simply to return a boolean value, treating "*" like any other bad
character. Special-case the handling of "*" (which only occurs in
very specific circumstances) at the caller. The resulting calling
code thereby also becomes more transparent.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index a615043..fd29d89 100644
--- a/refs.c
+++ b/refs.c
@@ -860,22 +860,21 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
* - it contains a "\" (backslash)
*/
+/* Return true iff ch is not allowed in reference names. */
static inline int bad_ref_char(int ch)
{
if (((unsigned) ch) <= ' ' || ch == 0x7f ||
ch == '~' || ch == '^' || ch == ':' || ch == '\\')
return 1;
/* 2.13 Pattern Matching Notation */
- if (ch == '?' || ch == '[') /* Unsupported */
+ if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
return 1;
- if (ch == '*') /* Supported at the end */
- return 2;
return 0;
}
int check_ref_format(const char *ref)
{
- int ch, level, bad_type, last;
+ int ch, level, last;
int ret = CHECK_REF_FORMAT_OK;
const char *cp = ref;
@@ -890,9 +889,8 @@ int check_ref_format(const char *ref)
/* we are at the beginning of the path component */
if (ch == '.')
return CHECK_REF_FORMAT_ERROR;
- bad_type = bad_ref_char(ch);
- if (bad_type) {
- if (bad_type == 2 && (!*cp || *cp == '/') &&
+ if (bad_ref_char(ch)) {
+ if (ch == '*' && (!*cp || *cp == '/') &&
ret == CHECK_REF_FORMAT_OK)
ret = CHECK_REF_FORMAT_WILDCARD;
else
@@ -902,8 +900,7 @@ int check_ref_format(const char *ref)
last = ch;
/* scan the rest of the path component */
while ((ch = *cp++) != 0) {
- bad_type = bad_ref_char(ch);
- if (bad_type)
+ if (bad_ref_char(ch))
return CHECK_REF_FORMAT_ERROR;
if (ch == '/')
break;
--
1.7.6.8.gd2879
^ permalink raw reply related
* Re: vcs-svn and friends
From: Stephen Bash @ 2011-09-15 21:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: David Michael Barr, Dmitry Ivankov, git, Junio C Hamano
In-Reply-To: <20110915204815.GA31508@elie>
----- Original Message -----
> From: "Jonathan Nieder" <jrnieder@gmail.com>
> Sent: Thursday, September 15, 2011 4:48:15 PM
> Subject: Re: vcs-svn and friends
>
> Stephen Bash wrote:
>
> > For those of us interested but out of the loop, does this mean you
> > have a working example where I can point it at a SVN repo and see
> > what happens? Having done our SVN to Git conversion last year, I
> > know our repo has a lot of the common SVN screw cases (non-branching
> > copies, partial merges, mis-merges, *lots* of retagging, changes
> > committed to tags, etc.) so if it's relatively easy to setup a test
> > I'm happy to run one.
>
> Thanks. It's very bare-bones at the moment: it just imports each
> revision as a whole tree, with no branch and merge tracking at all.
> So it would be very interesting to get this basic stuff out into the
> wild and then add some code implementing those things for you to break
> on top of it.
Okay, that matches my last known state of the project, guess I just got optimistic ;)
Since last summer I've been meaning to take a crack at svn-filter-root.py, but it keeps sliding off the priority list. If you guys don't beat me to it, maybe I'll eventually get around to it...
Thanks,
Stephen
^ permalink raw reply
* [PATCH v3 22/22] add_ref(): verify that the refname is formatted correctly
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
In add_ref(), verify that the refname is formatted correctly before
adding it to the ref_list. Here we have to allow refname components
that start with ".", since (for example) the remote protocol uses
synthetic reference name ".have". So add a new REFNAME_DOT_COMPONENT
flag that can be passed to check_refname_format() to allow leading
dots.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 18 ++++++++++++++----
refs.h | 6 +++++-
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 096b42c..832a52f 100644
--- a/refs.c
+++ b/refs.c
@@ -56,6 +56,8 @@ static struct ref_list *add_ref(const char *name, const unsigned char *sha1,
entry = xmalloc(sizeof(struct ref_list) + len);
hashcpy(entry->sha1, sha1);
hashclr(entry->peeled);
+ if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
+ die("Reference has invalid format: '%s'", name);
memcpy(entry->name, name, len);
entry->flag = flag;
entry->next = list;
@@ -900,7 +902,7 @@ static inline int bad_ref_char(int ch)
* the length of the component found, or -1 if the component is not
* legal.
*/
-static int check_refname_component(const char *ref)
+static int check_refname_component(const char *ref, int flags)
{
const char *cp;
char last = '\0';
@@ -919,8 +921,16 @@ static int check_refname_component(const char *ref)
}
if (cp == ref)
return -1; /* Component has zero length. */
- if (ref[0] == '.')
- return -1; /* Component starts with '.'. */
+ if (ref[0] == '.') {
+ if (!(flags & REFNAME_DOT_COMPONENT))
+ return -1; /* Component starts with '.'. */
+ /*
+ * Even if leading dots are allowed, don't allow "."
+ * as a component (".." is prevented by a rule above).
+ */
+ if (ref[1] == '\0')
+ return -1; /* Component equals ".". */
+ }
if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
return -1; /* Refname ends with ".lock". */
return cp - ref;
@@ -932,7 +942,7 @@ int check_refname_format(const char *ref, int flags)
while (1) {
/* We are at the start of a path component. */
- component_len = check_refname_component(ref);
+ component_len = check_refname_component(ref, flags);
if (component_len < 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
ref[0] == '*' &&
diff --git a/refs.h b/refs.h
index b0da5fc..d5ac133 100644
--- a/refs.h
+++ b/refs.h
@@ -99,6 +99,7 @@ extern int for_each_reflog(each_ref_fn, void *);
#define REFNAME_ALLOW_ONELEVEL 1
#define REFNAME_REFSPEC_PATTERN 2
+#define REFNAME_DOT_COMPONENT 4
/*
* Return 0 iff ref has the correct format for a refname according to
@@ -106,7 +107,10 @@ extern int for_each_reflog(each_ref_fn, void *);
* REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
* reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
* allow a "*" wildcard character in place of one of the name
- * components. No leading or repeated slashes are accepted.
+ * components. No leading or repeated slashes are accepted. If
+ * REFNAME_DOT_COMPONENT is set in flags, then allow refname
+ * components to start with "." (but not a whole component equal to
+ * "." or "..").
*/
extern int check_refname_format(const char *ref, int flags);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 21/22] resolve_ref(): expand documentation
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Record information about resolve_ref(), hard-won via reverse
engineering, in a comment for future spelunkers.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
cache.h | 34 +++++++++++++++++++++++++++++++++-
refs.c | 12 ------------
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/cache.h b/cache.h
index 607c2ea..aea8685 100644
--- a/cache.h
+++ b/cache.h
@@ -822,7 +822,39 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern int read_ref(const char *filename, unsigned char *sha1);
-extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
+
+/*
+ * Resolve a reference, recursively following symbolic refererences.
+ *
+ * Store the referred-to object's name in sha1 and return the name of
+ * the non-symbolic reference that ultimately pointed at it. The
+ * return value, if not NULL, is a pointer into either a static buffer
+ * or the input ref.
+ *
+ * If the reference cannot be resolved to an object, the behavior
+ * depends on the "reading" argument:
+ *
+ * - If reading is set, return NULL.
+ *
+ * - If reading is not set, clear sha1 and return the name of the last
+ * reference name in the chain, which will either be a non-symbolic
+ * reference or an undefined reference. If this is a prelude to
+ * "writing" to the ref, the return value is the name of the ref
+ * that will actually be created or changed.
+ *
+ * If flag is non-NULL, set the value that it points to the
+ * combination of REF_ISPACKED (if the reference was found among the
+ * packed references) and REF_ISSYMREF (if the initial reference was a
+ * symbolic reference).
+ *
+ * If ref is not a properly-formatted, normalized reference, return
+ * NULL. If more than MAXDEPTH recursive symbolic lookups are needed,
+ * give up and return NULL.
+ *
+ * errno is sometimes set on errors, but not always.
+ */
+extern const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag);
+
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
extern int interpret_branch_name(const char *str, struct strbuf *);
diff --git a/refs.c b/refs.c
index 0baa500..096b42c 100644
--- a/refs.c
+++ b/refs.c
@@ -482,18 +482,6 @@ static int get_packed_ref(const char *ref, unsigned char *sha1)
return -1;
}
-/*
- * If the "reading" argument is set, this function finds out what _object_
- * the ref points at by "reading" the ref. The ref, if it is not symbolic,
- * has to exist, and if it is symbolic, it has to point at an existing ref,
- * because the "read" goes through the symref to the ref it points at.
- *
- * The access that is not "reading" may often be "writing", but does not
- * have to; it can be merely checking _where it leads to_. If it is a
- * prelude to "writing" to the ref, a write to a symref that points at
- * yet-to-be-born ref will create the real ref pointed by the symref.
- * reading=0 allows the caller to check where such a symref leads to.
- */
const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag)
{
int depth = MAXDEPTH;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 18/22] resolve_ref(): verify that the input refname has the right format
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index b055501..ee3e0cc 100644
--- a/refs.c
+++ b/refs.c
@@ -504,6 +504,9 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (flag)
*flag = 0;
+ if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
+ return NULL;
+
for (;;) {
char path[PATH_MAX];
struct stat st;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 16/22] remote: use xstrdup() instead of strdup()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
remote.c | 2 +-
transport-helper.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index 6fcf809..e52aa9b 100644
--- a/remote.c
+++ b/remote.c
@@ -815,7 +815,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
refspec->dst, &ret))
return ret;
} else if (!strcmp(refspec->src, name))
- return strdup(refspec->dst);
+ return xstrdup(refspec->dst);
}
return NULL;
}
diff --git a/transport-helper.c b/transport-helper.c
index 4eab844..0713126 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -183,7 +183,7 @@ static struct child_process *get_helper(struct transport *transport)
ALLOC_GROW(refspecs,
refspec_nr + 1,
refspec_alloc);
- refspecs[refspec_nr++] = strdup(capname + strlen("refspec "));
+ refspecs[refspec_nr++] = xstrdup(capname + strlen("refspec "));
} else if (!strcmp(capname, "connect")) {
data->connect = 1;
} else if (!prefixcmp(capname, "export-marks ")) {
@@ -445,7 +445,7 @@ static int fetch_with_import(struct transport *transport,
if (data->refspecs)
private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
else
- private = strdup(posn->name);
+ private = xstrdup(posn->name);
read_ref(private, posn->old_sha1);
free(private);
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 10/22] resolve_ref(): explicitly fail if a symlink is not readable
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Previously the failure came later, after a few steps in which the
length was treated like the actual length of a string. Even though
the old code gave the same answers, it was somewhat misleading.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index d2aac24..c51fd45 100644
--- a/refs.c
+++ b/refs.c
@@ -518,6 +518,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
/* Follow "normalized" - ie "refs/.." symlinks by hand */
if (S_ISLNK(st.st_mode)) {
len = readlink(path, buffer, sizeof(buffer)-1);
+ if (len < 0)
+ return NULL;
if (len >= 5 && !memcmp("refs/", buffer, 5)) {
buffer[len] = 0;
strcpy(ref_buffer, buffer);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 14/22] resolve_ref(): extract a function get_packed_ref()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Making it a function and giving it a name makes the code clearer. I
also have a strong suspicion that the function will find other uses in
the future.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 47 ++++++++++++++++++++++++++++++++++-------------
1 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 79ab0eb..473f7f6 100644
--- a/refs.c
+++ b/refs.c
@@ -466,6 +466,23 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
}
/*
+ * Try to read ref from the packed references. On success, set sha1
+ * and return 0; otherwise, return -1.
+ */
+static int get_packed_ref(const char *ref, unsigned char *sha1)
+{
+ struct ref_list *list = get_packed_refs(NULL);
+ while (list) {
+ if (!strcmp(ref, list->name)) {
+ hashcpy(sha1, list->sha1);
+ return 0;
+ }
+ list = list->next;
+ }
+ return -1;
+}
+
+/*
* If the "reading" argument is set, this function finds out what _object_
* the ref points at by "reading" the ref. The ref, if it is not symbolic,
* has to exist, and if it is symbolic, it has to point at an existing ref,
@@ -497,22 +514,26 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return NULL;
git_snpath(path, sizeof(path), "%s", ref);
- /* Special case: non-existing file. */
+
if (lstat(path, &st) < 0) {
- struct ref_list *list = get_packed_refs(NULL);
- while (list) {
- if (!strcmp(ref, list->name)) {
- hashcpy(sha1, list->sha1);
- if (flag)
- *flag |= REF_ISPACKED;
- return ref;
- }
- list = list->next;
+ if (errno != ENOENT)
+ return NULL;
+ /*
+ * The loose reference file does not exist;
+ * check for a packed reference.
+ */
+ if (!get_packed_ref(ref, sha1)) {
+ if (flag)
+ *flag |= REF_ISPACKED;
+ return ref;
}
- if (reading || errno != ENOENT)
+ /* The reference is not a packed reference, either. */
+ if (reading) {
return NULL;
- hashclr(sha1);
- return ref;
+ } else {
+ hashclr(sha1);
+ return ref;
+ }
}
/* Follow "normalized" - ie "refs/.." symlinks by hand */
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 12/22] resolve_ref(): only follow a symlink that contains a valid, normalized refname
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index da9737f..8f0b871 100644
--- a/refs.c
+++ b/refs.c
@@ -521,7 +521,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (len < 0)
return NULL;
buffer[len] = 0;
- if (!prefixcmp(buffer, "refs/")) {
+ if (!prefixcmp(buffer, "refs/") &&
+ !check_refname_format(buffer, 0)) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 11/22] resolve_ref(): use prefixcmp()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Terminate the link content string one step earlier, allowing
prefixcmp() to be used instead of the less clear memcmp().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index c51fd45..da9737f 100644
--- a/refs.c
+++ b/refs.c
@@ -520,8 +520,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
len = readlink(path, buffer, sizeof(buffer)-1);
if (len < 0)
return NULL;
- if (len >= 5 && !memcmp("refs/", buffer, 5)) {
- buffer[len] = 0;
+ buffer[len] = 0;
+ if (!prefixcmp(buffer, "refs/")) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 13/22] resolve_ref(): turn buffer into a proper string as soon as possible
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Immediately strip off trailing spaces and null-terminate the string
holding the contents of the reference file; this allows the use of
string functions and avoids the need to keep separate track of the
string's length. (get_sha1_hex() fails automatically if the string is
too short.)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 8f0b871..79ab0eb 100644
--- a/refs.c
+++ b/refs.c
@@ -546,25 +546,25 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return NULL;
len = read_in_full(fd, buffer, sizeof(buffer)-1);
close(fd);
+ if (len < 0)
+ return NULL;
+ while (len && isspace(buffer[len-1]))
+ len--;
+ buffer[len] = '\0';
/*
* Is it a symbolic ref?
*/
- if (len < 4 || memcmp("ref:", buffer, 4))
+ if (prefixcmp(buffer, "ref:"))
break;
buf = buffer + 4;
- len -= 4;
- while (len && isspace(*buf))
- buf++, len--;
- while (len && isspace(buf[len-1]))
- len--;
- buf[len] = 0;
- memcpy(ref_buffer, buf, len + 1);
- ref = ref_buffer;
+ while (isspace(*buf))
+ buf++;
+ ref = strcpy(ref_buffer, buf);
if (flag)
*flag |= REF_ISSYMREF;
}
- if (len < 40 || get_sha1_hex(buffer, sha1))
+ if (get_sha1_hex(buffer, sha1))
return NULL;
return ref;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 15/22] resolve_ref(): do not follow incorrectly-formatted symbolic refs
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Emit a warning and fail if a symbolic reference refers to an
incorrectly-formatted refname.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index 473f7f6..b055501 100644
--- a/refs.c
+++ b/refs.c
@@ -581,6 +581,11 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
buf = buffer + 4;
while (isspace(*buf))
buf++;
+ if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
+ warning("symbolic reference in %s is formatted incorrectly",
+ path);
+ return NULL;
+ }
ref = strcpy(ref_buffer, buf);
if (flag)
*flag |= REF_ISSYMREF;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
While resolving references, if a reference is found that is in an
unrecognized format, emit a warning (and then fail, as before).
Wouldn't *you* want to know?
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index ee3e0cc..2387f4e 100644
--- a/refs.c
+++ b/refs.c
@@ -500,6 +500,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
ssize_t len;
char buffer[256];
static char ref_buffer[256];
+ char path[PATH_MAX];
if (flag)
*flag = 0;
@@ -508,7 +509,6 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return NULL;
for (;;) {
- char path[PATH_MAX];
struct stat st;
char *buf;
int fd;
@@ -593,8 +593,10 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (flag)
*flag |= REF_ISSYMREF;
}
- if (get_sha1_hex(buffer, sha1))
+ if (get_sha1_hex(buffer, sha1)) {
+ warning("reference in %s is formatted incorrectly", path);
return NULL;
+ }
return ref;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 17/22] remote: avoid passing NULL to read_ref()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
read_ref() can (and in test t5800, actually *does*) return NULL.
Don't pass the NULL along to read_ref(). Coincidentally, this mistake
didn't make resolve_ref() blow up, but upcoming changes to
resolve_ref() will make it less forgiving.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
transport-helper.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 0713126..6f227e2 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -446,8 +446,10 @@ static int fetch_with_import(struct transport *transport,
private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
else
private = xstrdup(posn->name);
- read_ref(private, posn->old_sha1);
- free(private);
+ if (private) {
+ read_ref(private, posn->old_sha1);
+ free(private);
+ }
}
strbuf_release(&buf);
return 0;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 20/22] resolve_ref(): also treat a too-long SHA1 as invalid
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
If the SHA1 in a reference file is not terminated by a space or
end-of-file, consider it malformed and emit a warning.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 2387f4e..0baa500 100644
--- a/refs.c
+++ b/refs.c
@@ -593,7 +593,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (flag)
*flag |= REF_ISSYMREF;
}
- if (get_sha1_hex(buffer, sha1)) {
+ /* Please note that FETCH_HEAD has a second line containing other data. */
+ if (get_sha1_hex(buffer, sha1) || (buffer[40] != '\0' && !isspace(buffer[40]))) {
warning("reference in %s is formatted incorrectly", path);
return NULL;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* Re: [PATCH/RFC] add lame win32 credential-helper
From: Jeff King @ 2011-09-15 21:40 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, jaysoffian, gitster
In-Reply-To: <1316118324-6164-1-git-send-email-kusmabite@gmail.com>
On Thu, Sep 15, 2011 at 10:25:24PM +0200, Erik Faye-Lund wrote:
> Not that it's useful yet, since the core-git code for the
> credential-helper support doesn't compile on Windows. So
> it's not fully tested, I've only read the interface
> documentation and experimented with it from the command
> line.
Which parts of the credential-helper code don't compile? I wouldn't be
surprised if the cache helper doesn't work (because it uses unix domain
sockets for communication). If it's too hard to adapt it to whatever IPC
mechanism would be appropriate on Windows, we can just leave it out on
that platform.
But the core code in git itself should be pretty straight forward. Let
me know what I can do to help.
-Peff
^ permalink raw reply
* Re: [PATCH/RFC] add lame win32 credential-helper
From: Erik Faye-Lund @ 2011-09-15 21:48 UTC (permalink / raw)
To: Jeff King; +Cc: git, jaysoffian, gitster
In-Reply-To: <20110915214026.GA18623@sigill.intra.peff.net>
On Thu, Sep 15, 2011 at 11:40 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Sep 15, 2011 at 10:25:24PM +0200, Erik Faye-Lund wrote:
>
>> Not that it's useful yet, since the core-git code for the
>> credential-helper support doesn't compile on Windows. So
>> it's not fully tested, I've only read the interface
>> documentation and experimented with it from the command
>> line.
>
> Which parts of the credential-helper code don't compile? I wouldn't be
> surprised if the cache helper doesn't work (because it uses unix domain
> sockets for communication).
That's exactly it:
unix-socket.c:12: warning: 'struct sockaddr_un' declared inside parameter list
unix-socket.c:12: warning: its scope is only this definition or declaration, whi
ch is probably not what you want
unix-socket.c: In function 'unix_sockaddr_init':
unix-socket.c:15: error: dereferencing pointer to incomplete type
unix-socket.c:17: error: dereferencing pointer to incomplete type
unix-socket.c:18: error: dereferencing pointer to incomplete type
unix-socket.c:19: error: dereferencing pointer to incomplete type
unix-socket.c: In function 'unix_stream_connect':
unix-socket.c:25: error: storage size of 'sa' isn't known
unix-socket.c: In function 'unix_stream_listen':
unix-socket.c:39: error: storage size of 'sa' isn't known
make: *** [unix-socket.o] Error 1
make: *** Waiting for unfinished jobs....
> If it's too hard to adapt it to whatever IPC
> mechanism would be appropriate on Windows, we can just leave it out on
> that platform.
>
> But the core code in git itself should be pretty straight forward.
I didn't mean that it was impossible to port, just that it didn't compile
as-is. I haven't looked into fixing up the code so it compiles on Windows
again myself. And I'm not really planning to; I have little git-time these
days, and little knowledge of how unix-sockets works...
^ 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