Git development
 help / color / mirror / Atom feed
* Re: git-p4 fails when cloning a p4 depo.
From: Benjamin Sergeant @ 2007-06-09  0:32 UTC (permalink / raw)
  To: hanwen; +Cc: git, Scott Lamb
In-Reply-To: <4669E73F.2040702@xs4all.nl>

On 6/8/07, Han-Wen Nienhuys <hanwen@xs4all.nl> wrote:
> Benjamin Sergeant escreveu:
>
> > So are you saying that in the old days, git-p4 was importing the p4
> > depo in small slices to not overkill the process memory (in case the
> > depo is big) ?
>
> no, in the "old days" git-p4 used a separate p4 invocation for each file.
>

Anyway, in case you hit command line lenght limit here it is. That
might be interesting for the "next days" :)

Benjamin.


[bsergean@flanders fast-export]$ git format-patch -k -m --stdout origin
From 45f2dbdb9a8c0b3beb007ae892613cdc4afab80a Mon Sep 17 00:00:00 2001
From: Benjamin Sergeant <bsergean@flanders.(none)>
Date: Fri, 8 Jun 2007 09:58:57 -0700
Subject: Split p4 print call into multiple call to not exceed the
command line lenght maximum

---
 git-p4 |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/git-p4 b/git-p4
index 36fe69a..906b193 100755
--- a/git-p4
+++ b/git-p4
@@ -703,9 +703,22 @@ class P4Sync(Command):
         if not files:
             return

-        filedata = p4CmdList('print %s' % ' '.join(['"%s#%s"' % (f['path'],
-                                                                 f['rev'])
-                                                    for f in files]))
+        # We cannot put all the files on the command line
+        # OS have limitations on the max lenght of arguments
+        # POSIX says it's 4096 bytes, default for Linux seems to be 130 K.
+        # and all OS from the table below seems to be higher than POSIX.
+        # See http://www.in-ulm.de/~mascheck/various/argmax/
+        chunk = ''
+        filedata = []
+        for i in xrange(len(files)):
+            f = files[i]
+            chunk += '"%s#%s" ' % (f['path'], f['rev'])
+            if len(chunk) > 4000 or i == len(files)-1:
+                data = p4CmdList('print %s' % chunk)
+                if "p4ExitCode" in data[0]:
+                    die("Problems executing p4. Error: [%d]." %
(data[0]['p4ExitCode']));
+                filedata.extend(data)
+                chunk = ''

         j = 0;
         contents = {}
@@ -1486,3 +1499,5 @@ def main():

 if __name__ == '__main__':
     main()
+
+# vim: set filetype=python sts=4 sw=4 et si :
--
1.5.0.4


>From dd9975708433efeec37b608755f54fbeaedf0f3f Mon Sep 17 00:00:00 2001
From: Benjamin Sergeant <bsergean@flanders.(none)>
Date: Fri, 8 Jun 2007 10:20:39 -0700
Subject: Use os.sysconf('SC_ARG_MAX') to retrieve the max value, and
build the string using join (faster)

---
 git-p4 |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/git-p4 b/git-p4
index 906b193..8dc1963 100755
--- a/git-p4
+++ b/git-p4
@@ -705,20 +705,23 @@ class P4Sync(Command):

         # We cannot put all the files on the command line
         # OS have limitations on the max lenght of arguments
-        # POSIX says it's 4096 bytes, default for Linux seems to be 130 K.
-        # and all OS from the table below seems to be higher than POSIX.
         # See http://www.in-ulm.de/~mascheck/various/argmax/
-        chunk = ''
+        chunks = []
+        chunkLenght = 0
         filedata = []
+        maxlenght = max(int(os.sysconf('SC_ARG_MAX') * 0.90), 4000)
+        print maxlenght
         for i in xrange(len(files)):
             f = files[i]
-            chunk += '"%s#%s" ' % (f['path'], f['rev'])
-            if len(chunk) > 4000 or i == len(files)-1:
-                data = p4CmdList('print %s' % chunk)
+            chunkLenght += len(f['path']) + len(f['rev'])
+            chunks.append('"%s#%s" ' % (f['path'], f['rev']))
+            if chunkLenght > maxlenght or i == len(files)-1:
+                data = p4CmdList('print %s' % ' '.join(chunks))
                 if "p4ExitCode" in data[0]:
                     die("Problems executing p4. Error: [%d]." %
(data[0]['p4ExitCode']));
                 filedata.extend(data)
-                chunk = ''
+                chunks = []
+                chunkLenght = 0

         j = 0;
         contents = {}
--
1.5.0.4

^ permalink raw reply related

* [PATCH 21/21] Add fsck_verify_ref_to_tag_object() to verify that refname matches name stored in tag object
From: Johan Herland @ 2007-06-09  0:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

On Monday 28 May 2007, Junio C Hamano wrote:
> However it would be a good
> idea to add logic to fsck to warn upon inconsistencis (perhaps
> by mistake) between refname and tag's true name.
>
> The check would say something like:
>
>   If an annotated (signed or unsigned) tag has a "tag"
>   line to give it the official $name, and if it is pointed
>   at by a ref, the refname must end with "/$name".
>   Otherwise we warn.
>
> Trivially, the above rule says that having v2.6.22 tag under
> refs/tags/v2.6.20 is a mistake we would want to be warned upon.

This patch adds the check described by Junio.

It also includes a bugfix when tag object parsing fails, from
Johannes Schindelin <Johannes.Schindelin@gmx.de>.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-fsck.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/builtin-fsck.c b/builtin-fsck.c
index 7b4c36b..a47976c 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -542,6 +542,30 @@ static int fsck_handle_reflog(const char *logname, const unsigned char *sha1, in
 	return 0;
 }
 
+static void fsck_verify_ref_to_tag_object(const char *refname, struct object *obj)
+{
+	/* Verify that refname matches the name stored in obj's "tag" header */
+	struct tag *tagobj = (struct tag *) parse_object(obj->sha1);
+	size_t tagname_len;
+	size_t refname_len = strlen(refname);
+
+	if (!tagobj->tag) {
+		error("%s: Failed to parse tag object %s", refname, sha1_to_hex(obj->sha1));
+		return;
+	}
+	tagname_len = strlen(tagobj->tag);
+	if (!tagname_len) return; /* No tag name stored in tagobj. Nothing to do. */
+
+	if (tagname_len < refname_len &&
+		!memcmp(tagobj->tag, refname + (refname_len - tagname_len), tagname_len) &&
+		refname[(refname_len - tagname_len) - 1] == '/') {
+		/* OK: tag name is "$name", and refname ends with "/$name" */
+		return;
+	}
+	else
+		error("%s: Mismatch between tag ref and tag object's name: '%s'", refname, tagobj->tag);
+}
+
 static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct object *obj;
@@ -556,6 +580,8 @@ static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int f
 		/* We'll continue with the rest despite the error.. */
 		return 0;
 	}
+	if (obj->type == OBJ_TAG) /* ref to tag object */
+		fsck_verify_ref_to_tag_object(refname, obj);
 	default_refs++;
 	obj->used = 1;
 	mark_reachable(obj, REACHABLE);
-- 
1.5.2

^ permalink raw reply related

* [PATCH 20/21] git-mktag tests: Expand on mktag selftests according to the new tag object structure
From: Johan Herland @ 2007-06-09  0:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Some more tests are added to test the new "keywords" header, and to test
the more thorough verification routine.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3800-mktag.sh |  212 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 200 insertions(+), 12 deletions(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 3381239..b3b3121 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -46,6 +46,8 @@ cat >tag.sig <<EOF
 xxxxxx 139e9b33986b1c2670fff52c5067603117b3e895
 type tag
 tag mytag
+tagger foo
+
 EOF
 
 cat >expect.pat <<EOF
@@ -61,6 +63,8 @@ cat >tag.sig <<EOF
 object zz9e9b33986b1c2670fff52c5067603117b3e895
 type tag
 tag mytag
+tagger foo
+
 EOF
 
 cat >expect.pat <<EOF
@@ -76,6 +80,8 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 xxxx tag
 tag mytag
+tagger foo
+
 EOF
 
 cat >expect.pat <<EOF
@@ -103,6 +109,8 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type tag
 xxx mytag
+tagger foo
+
 EOF
 
 cat >expect.pat <<EOF
@@ -118,6 +126,9 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type taggggggggggggggggggggggggggggggg
 tag
+keywords foo
+tagger bar@baz.com
+
 EOF
 
 cat >expect.pat <<EOF
@@ -127,13 +138,15 @@ EOF
 check_verify_failure '"tag" line label check #2'
 
 ############################################################
-#  8. type line type-name length check
+#  8. type line type name length check
 
 cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type taggggggggggggggggggggggggggggggg
 tag mytag
-tagger a
+keywords foo
+tagger bar@baz.com
+
 EOF
 
 cat >expect.pat <<EOF
@@ -149,7 +162,9 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type tagggg
 tag mytag
-tagger a
+keywords foo
+tagger bar@baz.com
+
 EOF
 
 cat >expect.pat <<EOF
@@ -159,13 +174,15 @@ EOF
 check_verify_failure 'verify object (SHA1/type) check'
 
 ############################################################
-# 10. verify tag-name check
+# 10. verify tag name check
 
 cat >tag.sig <<EOF
 object $head
 type commit
 tag my	tag
-tagger a
+keywords foo
+tagger bar@baz.com
+
 EOF
 
 cat >expect.pat <<EOF
@@ -175,28 +192,120 @@ EOF
 check_verify_failure 'verify tag-name check'
 
 ############################################################
-# 11. tagger line label check #1
+# 11. keywords line label check #1
 
 cat >tag.sig <<EOF
 object $head
 type commit
 tag mytag
+xxxxxxxx foo
+tagger bar@baz.com
+
 EOF
 
 cat >expect.pat <<EOF
-^error: .*char 70.*Could not find "tagger ".*$
+^error: .*char 70.*$
+EOF
+
+check_verify_failure '"keywords" line label check #1'
+
+############################################################
+# 12. keywords line label check #2
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+keywords
+tagger bar@baz.com
+
+EOF
+
+cat >expect.pat <<EOF
+^error: .*char 70.*$
+EOF
+
+check_verify_failure '"keywords" line label check #2'
+
+############################################################
+# 13. keywords line check #1
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+keywords foo bar	baz
+tagger bar@baz.com
+
+EOF
+
+cat >expect.pat <<EOF
+^error: .*char 82.*$
+EOF
+
+check_verify_failure '"keywords" line check #1'
+
+############################################################
+# 14. keywords line check #2
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+keywords foo,bar	baz
+tagger bar@baz.com
+
+EOF
+
+cat >expect.pat <<EOF
+^error: .*char 86.*$
+EOF
+
+check_verify_failure '"keywords" line check #2'
+
+############################################################
+# 15. keywords line check #3
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+keywords foo,,bar
+tagger bar@baz.com
+
+EOF
+
+cat >expect.pat <<EOF
+^error: .*char 82.*Found empty keyword.*$
+EOF
+
+check_verify_failure '"keywords" line check #3'
+
+############################################################
+# 16. tagger line label check #1
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+
+EOF
+
+cat >expect.pat <<EOF
+^error: .*char 70.*$
 EOF
 
 check_verify_failure '"tagger" line label check #1'
 
 ############################################################
-# 12. tagger line label check #2
+# 17. tagger line label check #2
 
 cat >tag.sig <<EOF
 object $head
 type commit
 tag mytag
-tagger
+xxxxxx bar@baz.com
+
 EOF
 
 cat >expect.pat <<EOF
@@ -206,25 +315,104 @@ EOF
 check_verify_failure '"tagger" line label check #2'
 
 ############################################################
-# 13. create valid tag
+# 18. tagger line label check #3
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+keywords foo
+tagger
+
+EOF
+
+cat >expect.pat <<EOF
+^error: .*char 83.*$
+EOF
+
+check_verify_failure '"tagger" line label check #3'
+
+############################################################
+# 19. create valid tag #1
 
 cat >tag.sig <<EOF
 object $head
 type commit
 tag mytag
 tagger another@example.com
+
 EOF
 
 test_expect_success \
-    'create valid tag' \
+    'create valid tag #1' \
     'git-mktag <tag.sig >.git/refs/tags/mytag 2>message'
 
 ############################################################
-# 14. check mytag
+# 20. check mytag
 
 test_expect_success \
     'check mytag' \
     'git-tag -l | grep mytag'
 
+############################################################
+# 21. create valid tag #2
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tagger another@example.com
+
+EOF
+
+test_expect_success \
+    'create valid tag #2' \
+    'git-mktag <tag.sig >.git/refs/tags/mytag 2>message'
+
+############################################################
+# 22. create valid tag #3
+
+cat >tag.sig <<EOF
+object $head
+type commit
+keywords foo,bar,baz,spam,spam,spam,spam,spam,spam,spam,spam
+tagger another@example.com
+
+EOF
+
+test_expect_success \
+    'create valid tag #3' \
+    'git-mktag <tag.sig >.git/refs/tags/mytag 2>message'
+
+############################################################
+# 23. create valid tag #4
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+keywords note
+tagger another@example.com
+
+EOF
+
+test_expect_success \
+    'create valid tag #4' \
+    'git-mktag <tag.sig >.git/refs/tags/mytag 2>message'
+
+############################################################
+# 24. create valid tag #5 (with empty message)
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+keywords note
+tagger a
+EOF
+
+test_expect_success \
+    'create valid tag #4' \
+    'git-mktag <tag.sig >.git/refs/tags/mytag 2>message'
+
 
 test_done
-- 
1.5.2

^ permalink raw reply related

* [PATCH 19/21] Documentation/git-mktag: Document the changes in tag object structure
From: Johan Herland @ 2007-06-09  0:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

The new structure of tag objects is documented.

Also some much-needed cleanup is done. E.g. remove the paragraph on the
8kB limit, since this limit was removed ages ago.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-mktag.txt |   41 ++++++++++++++++++++++++++++++-----------
 1 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-mktag.txt b/Documentation/git-mktag.txt
index 0ac3be1..411105d 100644
--- a/Documentation/git-mktag.txt
+++ b/Documentation/git-mktag.txt
@@ -8,38 +8,57 @@ git-mktag - Creates a tag object
 
 SYNOPSIS
 --------
-'git-mktag' < signature_file
+[verse]
+'git-mktag' < tag_data_file
+
 
 DESCRIPTION
 -----------
-Reads a tag contents on standard input and creates a tag object
+Reads tag object data on standard input and creates a tag object
 that can also be used to sign other objects.
 
 The output is the new tag's <object> identifier.
 
-Tag Format
+
+DISCUSSION
 ----------
-A tag signature file has a very simple fixed format: three lines of
+Tag object data has the following format
 
+[verse]
   object <sha1>
   type <typename>
-  tag <tagname>
+  tag <tagname>               (optional)
+  keywords <keywords>         (optional)
+  tagger <committer>
+
+followed by a blank line and a free-form message and an optional signature
+that git itself doesn't care about, but that may be verified with gpg or
+similar.
 
-followed by some 'optional' free-form signature that git itself
-doesn't care about, but that can be verified with gpg or similar.
+In the above listing, `<sha1>` represents the object pointed to by this tag,
+`<typename>` is the type of the object pointed to ("tag", "blob", "tree" or
+"commit"), `<tagname>` is the name of this tag object (and must correspond
+to the name of the corresponding ref (if any) in `.git/refs/`). `<keywords>`
+is a comma-separated list of keywords associated with this tag object, and
+`<committer>` holds the "`name <email>`" of the tag creator and timestamp
+of when the tag object was created (analogous to "committer" in commit
+objects).
 
-The size of the full object is artificially limited to 8kB.  (Just
-because I'm a lazy bastard, and if you can't fit a signature in that
-size, you're doing something wrong)
+If "`tag <tagname>`" is omitted, <tagname> defaults to the empty string.
+If "`keywords <keywords>`" is omitted, <keywords> defaults to "`tag`" if
+a <tagname> was given, "`note`" otherwise.
 
 
 Author
 ------
 Written by Linus Torvalds <torvalds@osdl.org>
 
+
 Documentation
 --------------
-Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
+Documentation by Johan Herland, David Greaves, Junio C Hamano and the
+git-list <git@vger.kernel.org>.
+
 
 GIT
 ---
-- 
1.5.2

^ permalink raw reply related

* [PATCH 18/21] git-fsck: Do thorough verification of tag objects
From: Johan Herland @ 2007-06-09  0:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Teach git-fsck to do the same kind of verification on tag objects that is
already done by git-mktag.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-fsck.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/builtin-fsck.c b/builtin-fsck.c
index 944a496..7b4c36b 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -359,11 +359,26 @@ static int fsck_commit(struct commit *commit)
 static int fsck_tag(struct tag *tag)
 {
 	struct object *tagged = tag->tagged;
+	enum object_type type;
+	unsigned long size;
+	char *data = (char *) read_sha1_file(tag->object.sha1, &type, &size);
 
 	if (verbose)
 		fprintf(stderr, "Checking tag %s\n",
 			sha1_to_hex(tag->object.sha1));
 
+	if (!data)
+		return objerror(&tag->object, "could not read tag");
+	if (type != OBJ_TAG) {
+		free(data);
+		return objerror(&tag->object, "not a tag (internal error)");
+	}
+	if (parse_and_verify_tag_buffer(0, data, size, 1)) { /* thoroughly verify tag object */
+		free(data);
+		return objerror(&tag->object, "failed thorough tag object verification");
+	}
+	free(data);
+
 	if (!tagged) {
 		return objerror(&tag->object, "could not load tagged object");
 	}
-- 
1.5.2

^ permalink raw reply related

* [PATCH 17/21] Update comments on tag objects in mktag.c
From: Johan Herland @ 2007-06-09  0:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Also update minimum tag object length to the new minimum length after refactoring.

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c |   30 ++++++++++++++++++++++--------
 tag.c   |    2 +-
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/mktag.c b/mktag.c
index 31eadd8..af0cfa6 100644
--- a/mktag.c
+++ b/mktag.c
@@ -2,16 +2,30 @@
 #include "tag.h"
 
 /*
- * A signature file has a very simple fixed format: four lines
- * of "object <sha1>" + "type <typename>" + "tag <tagname>" +
+ * Tag object data has the following format: two mandatory lines of
+ * "object <sha1>" + "type <typename>", plus two optional lines of
+ * "tag <tagname>" + "keywords <keywords>", plus a mandatory line of
  * "tagger <committer>", followed by a blank line, a free-form tag
- * message and a signature block that git itself doesn't care about,
- * but that can be verified with gpg or similar.
+ * message and an optional signature block that git itself doesn't
+ * care about, but that can be verified with gpg or similar.
  *
- * The first three lines are guaranteed to be at least 63 bytes:
- * "object <sha1>\n" is 48 bytes, "type tag\n" at 9 bytes is the
- * shortest possible type-line, and "tag .\n" at 6 bytes is the
- * shortest single-character-tag line.
+ * <sha1> represents the object pointed to by this tag, <typename> is
+ * the type of the object pointed to ("tag", "blob", "tree" or "commit"),
+ * <tagname> is the name of this tag object (and must correspond to the
+ * name of the corresponding ref (if any) in '.git/refs/'). <keywords> is
+ * a comma-separated list of keywords associated with this tag object, and
+ * <committer> holds the "name <email>" of the tag creator and timestamp
+ * of when the tag object was created (analogous to "committer" in commit
+ * objects).
+ *
+ * The first two lines are guaranteed to be at least 57 bytes:
+ * "object <sha1>\n" is 48 bytes, and "type tag\n" at 9 bytes is
+ * the shortest possible "type" line. The tagger line is at least
+ * "tagger \n" (8 bytes). Therefore a tag object _must_ have >= 65 bytes.
+ *
+ * If "tag <tagname>" is omitted, <tagname> defaults to the empty string.
+ * If "keywords <keywords>" is omitted, <keywords> defaults to "tag" if
+ * a <tagname> was given, "note" otherwise.
  */
 
 int main(int argc, char **argv)
diff --git a/tag.c b/tag.c
index 1caec19..af4356e 100644
--- a/tag.c
+++ b/tag.c
@@ -79,7 +79,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		item->object.parsed = 1;
 	}
 
-	if (size < 64)
+	if (size < 65)
 		return error("Tag object failed preliminary size check");
 
 	/* Verify mandatory object line */
-- 
1.5.2

^ permalink raw reply related

* [PATCH 16/21] Introduce optional "keywords" on tag objects
From: Johan Herland @ 2007-06-09  0:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

This patch introduces a new optional header line to the tag object, called
"keywords". The "keywords" line may contain a comma-separated list of
custom keywords associated with the tag object. There are two "special"
keywords, however: "tag" and "note": When the "keywords" header is
missing, its default value is set to "tag" if a "tag" header is
present; else the default "keywords" value is set to "note". The
"keywords" header is meant to be used by porcelains for classifying
different types of tag objects. This classification may then be used to
filter tag objects in the presentation layer (e.g. by implementing
extra filter options to --decorate, etc.)

Signed-off-by: Johan Herland <johan@herland.net>
---
 tag.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 tag.h |    1 +
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/tag.c b/tag.c
index fb678d7..1caec19 100644
--- a/tag.c
+++ b/tag.c
@@ -69,8 +69,8 @@ int parse_and_verify_tag_buffer(struct tag *item,
 
 	unsigned char sha1[20];
 	char type[20];
-	const char   *type_line, *tag_line, *tagger_line;
-	unsigned long type_len,   tag_len,   tagger_len;
+	const char   *type_line, *tag_line, *keywords_line, *tagger_line;
+	unsigned long type_len,   tag_len,   keywords_len,   tagger_len;
 	const char *header_end;
 
 	if (item) {
@@ -103,15 +103,26 @@ int parse_and_verify_tag_buffer(struct tag *item,
 			"Could not find \"\\n\" after \"type\"",
 			type_line - data);
 	if (prefixcmp(tag_line, "tag ")) /* no tag name given */
-		tagger_line = tag_line;
+		keywords_line = tag_line;
 	else {                           /* tag name given */
-		tagger_line = strchr(tag_line, '\n');
-		if (!tagger_line++)
+		keywords_line = strchr(tag_line, '\n');
+		if (!keywords_line++)
 			return error("Tag object (@ char " PD_FMT "): "
 				"Could not find \"\\n\" after \"tag\"",
 				tag_line - data);
 	}
 
+	/* Verify optional keywords line */
+	if (prefixcmp(keywords_line, "keywords ")) /* no keywords given */
+		tagger_line = keywords_line;
+	else {                                     /* keywords given */
+		tagger_line = strchr(keywords_line, '\n');
+		if (!tagger_line++)
+			return error("Tag object (@ char " PD_FMT "): "
+				"Could not find \"\\n\" after \"keywords\"",
+				keywords_line - data);
+	}
+
 	/*
 	 * Verify mandatory tagger line, but only when we're checking
 	 * thoroughly, i.e. on inserting a new tag, and on fsck.
@@ -142,8 +153,11 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	type_len       = tag_line > type_line ?
 			(tag_line - type_line) - 1 : 0;
 	tag_line      += strlen("tag ");
-	tag_len        = tagger_line > tag_line ?
-			(tagger_line - tag_line) - 1 : 0;
+	tag_len        = keywords_line > tag_line ?
+			(keywords_line - tag_line) - 1 : 0;
+	keywords_line += strlen("keywords ");
+	keywords_len   = tagger_line > keywords_line ?
+			(tagger_line - keywords_line) - 1 : 0;
 	tagger_line   += strlen("tagger ");
 	tagger_len     = header_end > tagger_line ?
 			(header_end - tagger_line) - 1 : 0;
@@ -176,6 +190,26 @@ int parse_and_verify_tag_buffer(struct tag *item,
 			}
 		}
 
+		/*
+		 * Verify keywords: disallow control characters, spaces,
+		 * or two subsequent commas
+		 */
+		if (keywords_len) { /* keywords line was given */
+			for (i = 0; i < keywords_len; ++i) {
+				unsigned char c = keywords_line[i];
+				if (c == ',' && keywords_line[i + 1] == ',')
+					/* consecutive commas */
+					return error("Tag object (@ char "
+						PD_FMT "): Found empty keyword",
+						keywords_line + i - data);
+				if (c > ' ' && c != 0x7f)
+					continue;
+				return error("Tag object (@ char " PD_FMT "): "
+					"Could not verify keywords",
+					keywords_line + i - data);
+			}
+		}
+
 		/* Verify tagger line */
 		/* TODO: check for committer/tagger info */
 
@@ -193,6 +227,18 @@ int parse_and_verify_tag_buffer(struct tag *item,
 			item->tag[0] = '\0';
 		}
 
+		if (keywords_len) { /* optional keywords string was given */
+			item->keywords = xmalloc(keywords_len + 1);
+			memcpy(item->keywords, keywords_line, keywords_len);
+			item->keywords[keywords_len] = '\0';
+		}
+		else { /* optional keywords string not given. Set default */
+			/* if tag name is set, use "tag"; else use "note" */
+			const char *default_kw = item->tag ? "tag" : "note";
+			item->keywords = xmalloc(strlen(default_kw) + 1);
+			memcpy(item->keywords, default_kw, strlen(default_kw) + 1);
+		}
+
 		if (!strcmp(type, blob_type)) {
 			item->tagged = &lookup_blob(sha1)->object;
 		} else if (!strcmp(type, tree_type)) {
diff --git a/tag.h b/tag.h
index 2631911..3b0008d 100644
--- a/tag.h
+++ b/tag.h
@@ -9,6 +9,7 @@ struct tag {
 	struct object object;
 	struct object *tagged;
 	char *tag;       /* optional, may be empty ("") */
+	char *keywords;  /* optional, defaults to tag ? "tag" : "note" */
 	char *signature; /* not actually implemented */
 };
 
-- 
1.5.2

^ permalink raw reply related

* [PATCH 15/21] Make tag names (i.e. the tag object's "tag" line) optional
From: Johan Herland @ 2007-06-09  0:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

The tag line is now optional. If not given in the tag object data, it
defaults to the empty string ("") in the parsed tag object.

The patch also adds a change to git-show; when asked to display a tag
object with no name (missing "tag" header), we will show the tag's sha1
instead of an empty string.

Finally the patch includes some tweaks to the selftests to make them work
with optional tag names.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-log.c    |    2 +-
 t/t3800-mktag.sh |    6 +++---
 tag.c            |   51 +++++++++++++++++++++++++++++----------------------
 tag.h            |    2 +-
 4 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 212cdfc..8a238c7 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -181,7 +181,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 			printf("%stag %s%s\n\n",
 					diff_get_color(rev.diffopt.color_diff,
 						DIFF_COMMIT),
-					t->tag,
+					*(t->tag) ? t->tag : name,
 					diff_get_color(rev.diffopt.color_diff,
 						DIFF_RESET));
 			ret = show_object(o->sha1, 1);
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 3bce5e0..3381239 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -106,7 +106,7 @@ xxx mytag
 EOF
 
 cat >expect.pat <<EOF
-^error: .*char 57.*"tag ".*$
+^error: .*char 57.*$
 EOF
 
 check_verify_failure '"tag" line label check #1'
@@ -121,7 +121,7 @@ tag
 EOF
 
 cat >expect.pat <<EOF
-^error: .*char 87.*"tag ".*$
+^error: .*char 87.*$
 EOF
 
 check_verify_failure '"tag" line label check #2'
@@ -169,7 +169,7 @@ tagger a
 EOF
 
 cat >expect.pat <<EOF
-^error: .*char 67.*Could not verify tag name.*$
+^error: .*char 66.*Could not verify tag name.*$
 EOF
 
 check_verify_failure 'verify tag-name check'
diff --git a/tag.c b/tag.c
index c373c86..fb678d7 100644
--- a/tag.c
+++ b/tag.c
@@ -96,15 +96,21 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		return error("Tag object (@ char 47): "
 			"Could not find \"\\ntype \"");
 
-	/* Verify mandatory tag line */
+	/* Verify optional tag line */
 	tag_line = strchr(type_line, '\n');
 	if (!tag_line++)
 		return error("Tag object (@ char " PD_FMT "): "
 			"Could not find \"\\n\" after \"type\"",
 			type_line - data);
-	if (prefixcmp(tag_line, "tag ") || tag_line[4] == '\n')
-		return error("Tag object (@ char " PD_FMT "): "
-			"Could not find \"tag \"", tag_line - data);
+	if (prefixcmp(tag_line, "tag ")) /* no tag name given */
+		tagger_line = tag_line;
+	else {                           /* tag name given */
+		tagger_line = strchr(tag_line, '\n');
+		if (!tagger_line++)
+			return error("Tag object (@ char " PD_FMT "): "
+				"Could not find \"\\n\" after \"tag\"",
+				tag_line - data);
+	}
 
 	/*
 	 * Verify mandatory tagger line, but only when we're checking
@@ -113,11 +119,6 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	 * notably the "v0.99" tag in the main git repo), and we don't
 	 * want to fail parsing on these.
 	 */
-	tagger_line = strchr(tag_line, '\n');
-	if (!tagger_line++)
-		return error("Tag object (@ char " PD_FMT "): "
-			"Could not find \"\\n\" after \"tag\"",
-			tag_line - data);
 	if (prefixcmp(tagger_line, "tagger ")) { /* no tagger given */
 		if (thorough_verify)
 			return error("Tag object (@ char " PD_FMT "): "
@@ -164,15 +165,15 @@ int parse_and_verify_tag_buffer(struct tag *item,
 				sha1_to_hex(sha1));
 
 		/* Verify tag name: disallow control characters or spaces */
-		for (i = 0;;) {
-			unsigned char c = tag_line[i++];
-			if (c == '\n')
-				break;
-			if (c > ' ')
-				continue;
-			return error("Tag object (@ char " PD_FMT "): "
-				"Could not verify tag name",
-				tag_line + i - data);
+		if (tag_len) { /* tag name was given */
+			for (i = 0; i < tag_len; ++i) {
+				unsigned char c = tag_line[i];
+				if (c > ' ' && c != 0x7f)
+					continue;
+				return error("Tag object (@ char " PD_FMT "): "
+					"Could not verify tag name",
+					tag_line + i - data);
+			}
 		}
 
 		/* Verify tagger line */
@@ -181,10 +182,16 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		/* The actual stuff afterwards we don't care about.. */
 	}
 
-	if (item) {
-		item->tag = xmalloc(tag_len + 1);
-		memcpy(item->tag, tag_line, tag_len);
-		item->tag[tag_len] = '\0';
+	if (item) { /* Store parsed information into item */
+		if (tag_len) { /* optional tag name was given */
+			item->tag = xmalloc(tag_len + 1);
+			memcpy(item->tag, tag_line, tag_len);
+			item->tag[tag_len] = '\0';
+		}
+		else { /* optional tag name not given */
+			item->tag = xmalloc(1);
+			item->tag[0] = '\0';
+		}
 
 		if (!strcmp(type, blob_type)) {
 			item->tagged = &lookup_blob(sha1)->object;
diff --git a/tag.h b/tag.h
index f341b7f..2631911 100644
--- a/tag.h
+++ b/tag.h
@@ -8,7 +8,7 @@ extern const char *tag_type;
 struct tag {
 	struct object object;
 	struct object *tagged;
-	char *tag;
+	char *tag;       /* optional, may be empty ("") */
 	char *signature; /* not actually implemented */
 };
 
-- 
1.5.2

^ permalink raw reply related

* [PATCH 14/21] Add proper parsing of "tagger" line, but only when thorough_verify is set
From: Johan Herland @ 2007-06-09  0:18 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Be explicit about the fact that the "tagger" line is now considered a
mandatory part of the tag object. There are however old tags (from before
July 2005) that don't have a "tagger" line. We therefore consider the
"tagger" line _optional_ when parsing tags without thorough_verify set.

In practice this means that verification of a missing "tagger" line will
only fail when adding or fsck-ing tags.

Signed-off-by: Johan Herland <johan@herland.net>
---
 tag.c |   48 ++++++++++++++++++++++++++++++++++--------------
 1 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/tag.c b/tag.c
index 9a6924f..c373c86 100644
--- a/tag.c
+++ b/tag.c
@@ -69,8 +69,9 @@ int parse_and_verify_tag_buffer(struct tag *item,
 
 	unsigned char sha1[20];
 	char type[20];
-	const char *type_line, *tag_line, *tagger_line;
-	unsigned long type_len, tag_len;
+	const char   *type_line, *tag_line, *tagger_line;
+	unsigned long type_len,   tag_len,   tagger_len;
+	const char *header_end;
 
 	if (item) {
 		if (item->object.parsed)
@@ -81,7 +82,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	if (size < 64)
 		return error("Tag object failed preliminary size check");
 
-	/* Verify object line */
+	/* Verify mandatory object line */
 	if (prefixcmp(data, "object "))
 		return error("Tag object (@ char 0): "
 			"Does not start with \"object \"");
@@ -89,13 +90,13 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	if (get_sha1_hex(data + 7, sha1))
 		return error("Tag object (@ char 7): Could not get SHA1 hash");
 
-	/* Verify type line */
+	/* Verify mandatory type line */
 	type_line = data + 48;
 	if (prefixcmp(type_line - 1, "\ntype "))
 		return error("Tag object (@ char 47): "
 			"Could not find \"\\ntype \"");
 
-	/* Verify tag-line */
+	/* Verify mandatory tag line */
 	tag_line = strchr(type_line, '\n');
 	if (!tag_line++)
 		return error("Tag object (@ char " PD_FMT "): "
@@ -105,27 +106,46 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		return error("Tag object (@ char " PD_FMT "): "
 			"Could not find \"tag \"", tag_line - data);
 
-	/* Verify the tagger line */
+	/*
+	 * Verify mandatory tagger line, but only when we're checking
+	 * thoroughly, i.e. on inserting a new tag, and on fsck.
+	 * There are existing tag objects without a tagger line (most
+	 * notably the "v0.99" tag in the main git repo), and we don't
+	 * want to fail parsing on these.
+	 */
 	tagger_line = strchr(tag_line, '\n');
 	if (!tagger_line++)
 		return error("Tag object (@ char " PD_FMT "): "
 			"Could not find \"\\n\" after \"tag\"",
 			tag_line - data);
-	if (thorough_verify) {
-		if (prefixcmp(tagger_line, "tagger ") || (tagger_line[7] == '\n'))
+	if (prefixcmp(tagger_line, "tagger ")) { /* no tagger given */
+		if (thorough_verify)
 			return error("Tag object (@ char " PD_FMT "): "
 				"Could not find \"tagger \"",
 				tagger_line - data);
+		header_end = tagger_line;
+	}
+	else {                                   /* tagger given */
+		header_end = strchr(tagger_line, '\n');
+		if (!header_end++)
+			return error("Tag object (@ char " PD_FMT "): "
+				"Could not find \"\\n\" after \"tagger\"",
+				tagger_line - data);
 	}
 
 	/*
 	 * Advance header field pointers past their initial identifier.
-	 * Calculate lengths of header fields.
+	 * Calculate lengths of header fields (0 for fields that are not given).
 	 */
-	type_line += strlen("type ");
-	type_len   = tag_line - type_line - 1;
-	tag_line  += strlen("tag ");
-	tag_len    = tagger_line - tag_line - 1;
+	type_line     += strlen("type ");
+	type_len       = tag_line > type_line ?
+			(tag_line - type_line) - 1 : 0;
+	tag_line      += strlen("tag ");
+	tag_len        = tagger_line > tag_line ?
+			(tagger_line - tag_line) - 1 : 0;
+	tagger_line   += strlen("tagger ");
+	tagger_len     = header_end > tagger_line ?
+			(header_end - tagger_line) - 1 : 0;
 
 	/* Get the actual type */
 	if (type_len >= sizeof(type))
@@ -155,7 +175,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 				tag_line + i - data);
 		}
 
-		/* Verify the tagger line */
+		/* Verify tagger line */
 		/* TODO: check for committer/tagger info */
 
 		/* The actual stuff afterwards we don't care about.. */
-- 
1.5.2

^ permalink raw reply related

* [PATCH 13/21] Collect skipping of header field names and calculation of line lengths in one place
From: Johan Herland @ 2007-06-09  0:18 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

For each of the parsed lines we at some point skip past its initial
identifier ("type ", "tag ", etc.). We also at some point calculate the
length of the remaining line. This patch moves these calculations into
one place. This provides _one_ place for all header lines where their
respective pointers start pointing at the header value (instead of the
start of the line), and their lengths are calculated.

Signed-off-by: Johan Herland <johan@herland.net>
---
 tag.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/tag.c b/tag.c
index ac76ec0..9a6924f 100644
--- a/tag.c
+++ b/tag.c
@@ -118,12 +118,20 @@ int parse_and_verify_tag_buffer(struct tag *item,
 				tagger_line - data);
 	}
 
+	/*
+	 * Advance header field pointers past their initial identifier.
+	 * Calculate lengths of header fields.
+	 */
+	type_line += strlen("type ");
+	type_len   = tag_line - type_line - 1;
+	tag_line  += strlen("tag ");
+	tag_len    = tagger_line - tag_line - 1;
+
 	/* Get the actual type */
-	type_len = tag_line - type_line - strlen("type \n");
 	if (type_len >= sizeof(type))
 		return error("Tag object (@ char " PD_FMT "): "
-			"Type too long", type_line + 5 - data);
-	memcpy(type, type_line + 5, type_len);
+			"Type too long", type_line - data);
+	memcpy(type, type_line, type_len);
 	type[type_len] = '\0';
 
 	if (thorough_verify) {
@@ -136,7 +144,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 				sha1_to_hex(sha1));
 
 		/* Verify tag name: disallow control characters or spaces */
-		for (i = 4;;) {
+		for (i = 0;;) {
 			unsigned char c = tag_line[i++];
 			if (c == '\n')
 				break;
@@ -154,9 +162,8 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	}
 
 	if (item) {
-		tag_len = tagger_line - tag_line - strlen("tag \n");
 		item->tag = xmalloc(tag_len + 1);
-		memcpy(item->tag, tag_line + 4, tag_len);
+		memcpy(item->tag, tag_line, tag_len);
 		item->tag[tag_len] = '\0';
 
 		if (!strcmp(type, blob_type)) {
@@ -169,7 +176,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 			item->tagged = &lookup_tag(sha1)->object;
 		} else {
 			error("Tag object (@ char " PD_FMT "): "
-				"Unknown type '%s'", type_line + 5 - data, type);
+				"Unknown type '%s'", type_line - data, type);
 			item->tagged = NULL;
 		}
 
-- 
1.5.2

^ permalink raw reply related

* [PATCH 12/21] Use prefixcmp() instead of memcmp() for cleaner code with less magic numbers
From: Johan Herland @ 2007-06-09  0:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 tag.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/tag.c b/tag.c
index c7c75b3..ac76ec0 100644
--- a/tag.c
+++ b/tag.c
@@ -51,6 +51,13 @@ static int verify_object(unsigned char *sha1, const char *expected_type)
 	return ret;
 }
 
+/*
+ * Perform parsing and verification of tag object data.
+ *
+ * The 'item' parameter may be set to NULL if only verification is desired.
+ *
+ * The given data _must_ be null-terminated.
+ */
 int parse_and_verify_tag_buffer(struct tag *item,
 		const char *data, const unsigned long size, int thorough_verify)
 {
@@ -75,7 +82,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		return error("Tag object failed preliminary size check");
 
 	/* Verify object line */
-	if (memcmp(data, "object ", 7))
+	if (prefixcmp(data, "object "))
 		return error("Tag object (@ char 0): "
 			"Does not start with \"object \"");
 
@@ -84,7 +91,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 
 	/* Verify type line */
 	type_line = data + 48;
-	if (memcmp(type_line - 1, "\ntype ", 6))
+	if (prefixcmp(type_line - 1, "\ntype "))
 		return error("Tag object (@ char 47): "
 			"Could not find \"\\ntype \"");
 
@@ -94,7 +101,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		return error("Tag object (@ char " PD_FMT "): "
 			"Could not find \"\\n\" after \"type\"",
 			type_line - data);
-	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
+	if (prefixcmp(tag_line, "tag ") || tag_line[4] == '\n')
 		return error("Tag object (@ char " PD_FMT "): "
 			"Could not find \"tag \"", tag_line - data);
 
@@ -105,7 +112,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 			"Could not find \"\\n\" after \"tag\"",
 			tag_line - data);
 	if (thorough_verify) {
-		if (memcmp(tagger_line, "tagger ", 7) || (tagger_line[7] == '\n'))
+		if (prefixcmp(tagger_line, "tagger ") || (tagger_line[7] == '\n'))
 			return error("Tag object (@ char " PD_FMT "): "
 				"Could not find \"tagger \"",
 				tagger_line - data);
-- 
1.5.2

^ permalink raw reply related

* [PATCH 11/21] Rewrite error messages; fix up line lengths
From: Johan Herland @ 2007-06-09  0:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Also update selftests to reflect new error messages.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3800-mktag.sh |   22 +++++++++++-----------
 tag.c            |   44 +++++++++++++++++++++++++++++---------------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 0e87d2a..3bce5e0 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -49,7 +49,7 @@ tag mytag
 EOF
 
 cat >expect.pat <<EOF
-^error: char0: .*"object "$
+^error: .*char 0.*"object ".*$
 EOF
 
 check_verify_failure '"object" line label check'
@@ -64,7 +64,7 @@ tag mytag
 EOF
 
 cat >expect.pat <<EOF
-^error: char7: .*SHA1 hash$
+^error: .*char 7.*SHA1 hash.*$
 EOF
 
 check_verify_failure '"object" line SHA1 check'
@@ -79,7 +79,7 @@ tag mytag
 EOF
 
 cat >expect.pat <<EOF
-^error: char47: .*"[\]ntype "$
+^error: .*char 47.*"[\]ntype ".*$
 EOF
 
 check_verify_failure '"type" line label check'
@@ -91,7 +91,7 @@ echo "object 779e9b33986b1c2670fff52c5067603117b3e895" >tag.sig
 printf "type tagsssssssssssssssssssssssssssssss" >>tag.sig
 
 cat >expect.pat <<EOF
-^error: char48: .*"[\]n"$
+^error: .*char 48.*"[\]n".*$
 EOF
 
 check_verify_failure '"type" line eol check'
@@ -106,7 +106,7 @@ xxx mytag
 EOF
 
 cat >expect.pat <<EOF
-^error: char57: no "tag " found$
+^error: .*char 57.*"tag ".*$
 EOF
 
 check_verify_failure '"tag" line label check #1'
@@ -121,7 +121,7 @@ tag
 EOF
 
 cat >expect.pat <<EOF
-^error: char87: no "tag " found$
+^error: .*char 87.*"tag ".*$
 EOF
 
 check_verify_failure '"tag" line label check #2'
@@ -137,7 +137,7 @@ tagger a
 EOF
 
 cat >expect.pat <<EOF
-^error: char53: type too long$
+^error: .*char 53.*Type too long.*$
 EOF
 
 check_verify_failure '"type" line type-name length check'
@@ -153,7 +153,7 @@ tagger a
 EOF
 
 cat >expect.pat <<EOF
-^error: char7: could not verify object.*$
+^error: .*char 7.*Could not verify tagged object.*$
 EOF
 
 check_verify_failure 'verify object (SHA1/type) check'
@@ -169,7 +169,7 @@ tagger a
 EOF
 
 cat >expect.pat <<EOF
-^error: char67: could not verify tag name$
+^error: .*char 67.*Could not verify tag name.*$
 EOF
 
 check_verify_failure 'verify tag-name check'
@@ -184,7 +184,7 @@ tag mytag
 EOF
 
 cat >expect.pat <<EOF
-^error: char70: could not find "tagger"$
+^error: .*char 70.*Could not find "tagger ".*$
 EOF
 
 check_verify_failure '"tagger" line label check #1'
@@ -200,7 +200,7 @@ tagger
 EOF
 
 cat >expect.pat <<EOF
-^error: char70: could not find "tagger"$
+^error: .*char 70.*Could not find "tagger ".*$
 EOF
 
 check_verify_failure '"tagger" line label check #2'
diff --git a/tag.c b/tag.c
index e12b9fc..c7c75b3 100644
--- a/tag.c
+++ b/tag.c
@@ -72,41 +72,50 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	}
 
 	if (size < 64)
-		return error("failed preliminary size check");
+		return error("Tag object failed preliminary size check");
 
 	/* Verify object line */
 	if (memcmp(data, "object ", 7))
-		return error("char%d: does not start with \"object \"", 0);
+		return error("Tag object (@ char 0): "
+			"Does not start with \"object \"");
 
 	if (get_sha1_hex(data + 7, sha1))
-		return error("char%d: could not get SHA1 hash", 7);
+		return error("Tag object (@ char 7): Could not get SHA1 hash");
 
 	/* Verify type line */
 	type_line = data + 48;
 	if (memcmp(type_line - 1, "\ntype ", 6))
-		return error("char%d: could not find \"\\ntype \"", 47);
+		return error("Tag object (@ char 47): "
+			"Could not find \"\\ntype \"");
 
 	/* Verify tag-line */
 	tag_line = strchr(type_line, '\n');
-	if (!tag_line)
-		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - data);
-	tag_line++;
+	if (!tag_line++)
+		return error("Tag object (@ char " PD_FMT "): "
+			"Could not find \"\\n\" after \"type\"",
+			type_line - data);
 	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
-		return error("char" PD_FMT ": no \"tag \" found", tag_line - data);
+		return error("Tag object (@ char " PD_FMT "): "
+			"Could not find \"tag \"", tag_line - data);
 
 	/* Verify the tagger line */
 	tagger_line = strchr(tag_line, '\n');
 	if (!tagger_line++)
-		return error("char" PD_FMT ": could not find next \"\\n\"", tag_line - data);
+		return error("Tag object (@ char " PD_FMT "): "
+			"Could not find \"\\n\" after \"tag\"",
+			tag_line - data);
 	if (thorough_verify) {
 		if (memcmp(tagger_line, "tagger ", 7) || (tagger_line[7] == '\n'))
-			return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - data);
+			return error("Tag object (@ char " PD_FMT "): "
+				"Could not find \"tagger \"",
+				tagger_line - data);
 	}
 
 	/* Get the actual type */
 	type_len = tag_line - type_line - strlen("type \n");
 	if (type_len >= sizeof(type))
-		return error("char" PD_FMT ": type too long", type_line + 5 - data);
+		return error("Tag object (@ char " PD_FMT "): "
+			"Type too long", type_line + 5 - data);
 	memcpy(type, type_line + 5, type_len);
 	type[type_len] = '\0';
 
@@ -115,16 +124,20 @@ int parse_and_verify_tag_buffer(struct tag *item,
 
 		/* Verify that the object matches */
 		if (verify_object(sha1, type))
-			return error("char%d: could not verify object %s", 7, sha1_to_hex(sha1));
+			return error("Tag object (@ char 7): "
+				"Could not verify tagged object %s",
+				sha1_to_hex(sha1));
 
-		/* Verify the tag-name: we don't allow control characters or spaces in it */
+		/* Verify tag name: disallow control characters or spaces */
 		for (i = 4;;) {
 			unsigned char c = tag_line[i++];
 			if (c == '\n')
 				break;
 			if (c > ' ')
 				continue;
-			return error("char" PD_FMT ": could not verify tag name", tag_line + i - data);
+			return error("Tag object (@ char " PD_FMT "): "
+				"Could not verify tag name",
+				tag_line + i - data);
 		}
 
 		/* Verify the tagger line */
@@ -148,7 +161,8 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		} else if (!strcmp(type, tag_type)) {
 			item->tagged = &lookup_tag(sha1)->object;
 		} else {
-			error("Unknown type %s", type);
+			error("Tag object (@ char " PD_FMT "): "
+				"Unknown type '%s'", type_line + 5 - data, type);
 			item->tagged = NULL;
 		}
 
-- 
1.5.2

^ permalink raw reply related

* [PATCH 10/21] Free mktag's buffer before dying
From: Johan Herland @ 2007-06-09  0:16 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/mktag.c b/mktag.c
index 4f226ab..31eadd8 100644
--- a/mktag.c
+++ b/mktag.c
@@ -21,7 +21,7 @@ int main(int argc, char **argv)
 	unsigned char result_sha1[20];
 
 	if (argc != 1)
-		usage("git-mktag < signaturefile");
+		usage("git-mktag < tag_data_file");
 
 	setup_git_directory();
 
@@ -32,14 +32,17 @@ int main(int argc, char **argv)
 	buffer[size] = 0;
 
 	/* Verify tag object data */
-	if (parse_and_verify_tag_buffer(0, buffer, size, 1))
-		die("invalid tag signature file");
+	if (parse_and_verify_tag_buffer(0, buffer, size, 1)) {
+		free(buffer);
+		die("invalid tag data file");
+	}
 
-	if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
+	if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {
+		free(buffer);
 		die("unable to write tag file");
+	}
 
 	free(buffer);
-
 	printf("%s\n", sha1_to_hex(result_sha1));
 	return 0;
 }
-- 
1.5.2

^ permalink raw reply related

* [PATCH 09/21] Remove unneeded code from mktag.c
From: Johan Herland @ 2007-06-09  0:16 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c |   94 ---------------------------------------------------------------
 1 files changed, 0 insertions(+), 94 deletions(-)

diff --git a/mktag.c b/mktag.c
index 5780f33..4f226ab 100644
--- a/mktag.c
+++ b/mktag.c
@@ -14,100 +14,6 @@
  * shortest single-character-tag line.
  */
 
-/*
- * We refuse to tag something we can't verify. Just because.
- */
-static int verify_object(unsigned char *sha1, const char *expected_type)
-{
-	int ret = -1;
-	enum object_type type;
-	unsigned long size;
-	void *buffer = read_sha1_file(sha1, &type, &size);
-
-	if (buffer) {
-		if (type == type_from_string(expected_type))
-			ret = check_sha1_signature(sha1, buffer, size, expected_type);
-		free(buffer);
-	}
-	return ret;
-}
-
-static int verify_tag(char *data, unsigned long size)
-{
-#ifdef NO_C99_FORMAT
-#define PD_FMT "%d"
-#else
-#define PD_FMT "%td"
-#endif
-
-	unsigned char sha1[20];
-	char type[20];
-	const char *type_line, *tag_line, *tagger_line;
-	unsigned long type_len;
-
-	if (size < 64)
-		return error("wanna fool me ? you obviously got the size wrong !");
-
-	/* Verify object line */
-	if (memcmp(data, "object ", 7))
-		return error("char%d: does not start with \"object \"", 0);
-
-	if (get_sha1_hex(data + 7, sha1))
-		return error("char%d: could not get SHA1 hash", 7);
-
-	/* Verify type line */
-	type_line = data + 48;
-	if (memcmp(type_line - 1, "\ntype ", 6))
-		return error("char%d: could not find \"\\ntype \"", 47);
-
-	/* Verify tag-line */
-	tag_line = strchr(type_line, '\n');
-	if (!tag_line++)
-		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - data);
-	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
-		return error("char" PD_FMT ": no \"tag \" found", tag_line - data);
-
-	/* Verify the tagger line */
-	tagger_line = strchr(tag_line, '\n');
-	if (!tagger_line++)
-		return error("char" PD_FMT ": could not find next \"\\n\"", tag_line - data);
-	if (memcmp(tagger_line, "tagger ", 7) || (tagger_line[7] == '\n'))
-		return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - data);
-
-	/* Get the actual type */
-	type_len = tag_line - type_line - strlen("type \n");
-	if (type_len >= sizeof(type))
-		return error("char" PD_FMT ": type too long", type_line + 5 - data);
-	memcpy(type, type_line + 5, type_len);
-	type[type_len] = '\0';
-
-	{
-		unsigned long i;
-
-		/* Verify that the object matches */
-		if (verify_object(sha1, type))
-			return error("char%d: could not verify object %s", 7, sha1_to_hex(sha1));
-
-		/* Verify the tag-name: we don't allow control characters or spaces in it */
-		for (i = 4;;) {
-			unsigned char c = tag_line[i++];
-			if (c == '\n')
-				break;
-			if (c > ' ')
-				continue;
-			return error("char" PD_FMT ": could not verify tag name", tag_line + i - data);
-		}
-	}
-
-	/* TODO: check for committer info + blank line? */
-	/* Also, the minimum length is probably + "tagger .", or 63+8=71 */
-
-	/* The actual stuff afterwards we don't care about.. */
-	return 0;
-
-#undef PD_FMT
-}
-
 int main(int argc, char **argv)
 {
 	unsigned long size = 4096;
-- 
1.5.2

^ permalink raw reply related

* [PATCH 08/21] Switch from verify_tag() to parse_and_verify_tag_buffer() for verifying tag objects in git-mktag
From: Johan Herland @ 2007-06-09  0:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

This involves exposing parse_and_verify_tag_buffer() in the tag API
(tag.h).

Also synchronize selftest with change in error message.

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c          |    5 ++---
 t/t3800-mktag.sh |    2 +-
 tag.c            |    2 +-
 tag.h            |    2 ++
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/mktag.c b/mktag.c
index 2e70504..5780f33 100644
--- a/mktag.c
+++ b/mktag.c
@@ -125,9 +125,8 @@ int main(int argc, char **argv)
 	}
 	buffer[size] = 0;
 
-	/* Verify it for some basic sanity: it needs to start with
-	   "object <sha1>\ntype\ntagger " */
-	if (verify_tag(buffer, size) < 0)
+	/* Verify tag object data */
+	if (parse_and_verify_tag_buffer(0, buffer, size, 1))
 		die("invalid tag signature file");
 
 	if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index b4edb4d..0e87d2a 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -34,7 +34,7 @@ too short for a tag
 EOF
 
 cat >expect.pat <<EOF
-^error: .*size wrong.*$
+^error: .*size.*$
 EOF
 
 check_verify_failure 'Tag object length check'
diff --git a/tag.c b/tag.c
index 3896e45..e12b9fc 100644
--- a/tag.c
+++ b/tag.c
@@ -51,7 +51,7 @@ static int verify_object(unsigned char *sha1, const char *expected_type)
 	return ret;
 }
 
-static int parse_and_verify_tag_buffer(struct tag *item,
+int parse_and_verify_tag_buffer(struct tag *item,
 		const char *data, const unsigned long size, int thorough_verify)
 {
 #ifdef NO_C99_FORMAT
diff --git a/tag.h b/tag.h
index 7a0cb00..f341b7f 100644
--- a/tag.h
+++ b/tag.h
@@ -13,6 +13,8 @@ struct tag {
 };
 
 extern struct tag *lookup_tag(const unsigned char *sha1);
+extern int parse_and_verify_tag_buffer(struct tag *item,
+	const char *data, const unsigned long size, int thorough_verify);
 extern int parse_tag_buffer(struct tag *item, void *data, unsigned long size);
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
-- 
1.5.2

^ permalink raw reply related

* [PATCH 07/21] Copy the remaining differences from verify_tag() to parse_tag_buffer_internal()
From: Johan Herland @ 2007-06-09  0:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Rename parse_tag_buffer_internal() to parse_and_verify_tag_buffer() since
it now does tag object verification as well.

Add a new parameter 'thorough_verify' for turning on/off the extra code
to be run when verifying tag objects (as opposed to general parsing).

verify_tag() and parse_and_verify_tag_buffer() are now functionally
equivalent, provided that parse_and_verify_tag_buffer() is called with
item == NULL and thorough_verification != 0.

Signed-off-by: Johan Herland <johan@herland.net>
---
 tag.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/tag.c b/tag.c
index b134967..3896e45 100644
--- a/tag.c
+++ b/tag.c
@@ -33,7 +33,26 @@ struct tag *lookup_tag(const unsigned char *sha1)
         return (struct tag *) obj;
 }
 
-static int parse_tag_buffer_internal(struct tag *item, const char *data, const unsigned long size)
+/*
+ * We refuse to tag something we can't verify. Just because.
+ */
+static int verify_object(unsigned char *sha1, const char *expected_type)
+{
+	int ret = -1;
+	enum object_type type;
+	unsigned long size;
+	void *buffer = read_sha1_file(sha1, &type, &size);
+
+	if (buffer) {
+		if (type == type_from_string(expected_type))
+			ret = check_sha1_signature(sha1, buffer, size, expected_type);
+		free(buffer);
+	}
+	return ret;
+}
+
+static int parse_and_verify_tag_buffer(struct tag *item,
+		const char *data, const unsigned long size, int thorough_verify)
 {
 #ifdef NO_C99_FORMAT
 #define PD_FMT "%d"
@@ -79,6 +98,10 @@ static int parse_tag_buffer_internal(struct tag *item, const char *data, const u
 	tagger_line = strchr(tag_line, '\n');
 	if (!tagger_line++)
 		return error("char" PD_FMT ": could not find next \"\\n\"", tag_line - data);
+	if (thorough_verify) {
+		if (memcmp(tagger_line, "tagger ", 7) || (tagger_line[7] == '\n'))
+			return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - data);
+	}
 
 	/* Get the actual type */
 	type_len = tag_line - type_line - strlen("type \n");
@@ -87,6 +110,29 @@ static int parse_tag_buffer_internal(struct tag *item, const char *data, const u
 	memcpy(type, type_line + 5, type_len);
 	type[type_len] = '\0';
 
+	if (thorough_verify) {
+		unsigned long i;
+
+		/* Verify that the object matches */
+		if (verify_object(sha1, type))
+			return error("char%d: could not verify object %s", 7, sha1_to_hex(sha1));
+
+		/* Verify the tag-name: we don't allow control characters or spaces in it */
+		for (i = 4;;) {
+			unsigned char c = tag_line[i++];
+			if (c == '\n')
+				break;
+			if (c > ' ')
+				continue;
+			return error("char" PD_FMT ": could not verify tag name", tag_line + i - data);
+		}
+
+		/* Verify the tagger line */
+		/* TODO: check for committer/tagger info */
+
+		/* The actual stuff afterwards we don't care about.. */
+	}
+
 	if (item) {
 		tag_len = tagger_line - tag_line - strlen("tag \n");
 		item->tag = xmalloc(tag_len + 1);
@@ -120,7 +166,7 @@ static int parse_tag_buffer_internal(struct tag *item, const char *data, const u
 
 int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 {
-	return parse_tag_buffer_internal(item, (const char *) data, size);
+	return parse_and_verify_tag_buffer(item, (const char *) data, size, 0);
 }
 
 int parse_tag(struct tag *item)
-- 
1.5.2

^ permalink raw reply related

* [PATCH 06/21] Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line'
From: Johan Herland @ 2007-06-09  0:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/mktag.c b/mktag.c
index 4dbefab..2e70504 100644
--- a/mktag.c
+++ b/mktag.c
@@ -81,19 +81,22 @@ static int verify_tag(char *data, unsigned long size)
 	memcpy(type, type_line + 5, type_len);
 	type[type_len] = '\0';
 
-	/* Verify that the object matches */
-	if (verify_object(sha1, type))
-		return error("char%d: could not verify object %s", 7, sha1_to_hex(sha1));
-
-	/* Verify the tag-name: we don't allow control characters or spaces in it */
-	tag_line += 4;
-	for (;;) {
-		unsigned char c = *tag_line++;
-		if (c == '\n')
-			break;
-		if (c > ' ')
-			continue;
-		return error("char" PD_FMT ": could not verify tag name", tag_line - data);
+	{
+		unsigned long i;
+
+		/* Verify that the object matches */
+		if (verify_object(sha1, type))
+			return error("char%d: could not verify object %s", 7, sha1_to_hex(sha1));
+
+		/* Verify the tag-name: we don't allow control characters or spaces in it */
+		for (i = 4;;) {
+			unsigned char c = tag_line[i++];
+			if (c == '\n')
+				break;
+			if (c > ' ')
+				continue;
+			return error("char" PD_FMT ": could not verify tag name", tag_line + i - data);
+		}
 	}
 
 	/* TODO: check for committer info + blank line? */
-- 
1.5.2

^ permalink raw reply related

* [PATCH 05/21] Make parse_tag_buffer_internal() handle item == NULL
From: Johan Herland @ 2007-06-09  0:14 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

This is in preparation for unifying verify_tag() and
parse_tag_buffer_internal().

Signed-off-by: Johan Herland <johan@herland.net>
---
 tag.c |   54 +++++++++++++++++++++++++++++-------------------------
 1 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/tag.c b/tag.c
index 19c66cd..b134967 100644
--- a/tag.c
+++ b/tag.c
@@ -46,9 +46,11 @@ static int parse_tag_buffer_internal(struct tag *item, const char *data, const u
 	const char *type_line, *tag_line, *tagger_line;
 	unsigned long type_len, tag_len;
 
-	if (item->object.parsed)
-		return 0;
-	item->object.parsed = 1;
+	if (item) {
+		if (item->object.parsed)
+			return 0;
+		item->object.parsed = 1;
+	}
 
 	if (size < 64)
 		return error("failed preliminary size check");
@@ -85,28 +87,30 @@ static int parse_tag_buffer_internal(struct tag *item, const char *data, const u
 	memcpy(type, type_line + 5, type_len);
 	type[type_len] = '\0';
 
-	tag_len = tagger_line - tag_line - strlen("tag \n");
-	item->tag = xmalloc(tag_len + 1);
-	memcpy(item->tag, tag_line + 4, tag_len);
-	item->tag[tag_len] = '\0';
-
-	if (!strcmp(type, blob_type)) {
-		item->tagged = &lookup_blob(sha1)->object;
-	} else if (!strcmp(type, tree_type)) {
-		item->tagged = &lookup_tree(sha1)->object;
-	} else if (!strcmp(type, commit_type)) {
-		item->tagged = &lookup_commit(sha1)->object;
-	} else if (!strcmp(type, tag_type)) {
-		item->tagged = &lookup_tag(sha1)->object;
-	} else {
-		error("Unknown type %s", type);
-		item->tagged = NULL;
-	}
-
-	if (item->tagged && track_object_refs) {
-		struct object_refs *refs = alloc_object_refs(1);
-		refs->ref[0] = item->tagged;
-		set_object_refs(&item->object, refs);
+	if (item) {
+		tag_len = tagger_line - tag_line - strlen("tag \n");
+		item->tag = xmalloc(tag_len + 1);
+		memcpy(item->tag, tag_line + 4, tag_len);
+		item->tag[tag_len] = '\0';
+
+		if (!strcmp(type, blob_type)) {
+			item->tagged = &lookup_blob(sha1)->object;
+		} else if (!strcmp(type, tree_type)) {
+			item->tagged = &lookup_tree(sha1)->object;
+		} else if (!strcmp(type, commit_type)) {
+			item->tagged = &lookup_commit(sha1)->object;
+		} else if (!strcmp(type, tag_type)) {
+			item->tagged = &lookup_tag(sha1)->object;
+		} else {
+			error("Unknown type %s", type);
+			item->tagged = NULL;
+		}
+
+		if (item->tagged && track_object_refs) {
+			struct object_refs *refs = alloc_object_refs(1);
+			refs->ref[0] = item->tagged;
+			set_object_refs(&item->object, refs);
+		}
 	}
 
 	return 0;
-- 
1.5.2

^ permalink raw reply related

* [PATCH 04/21] Refactor verification of "tagger" line to be more similar to verification of "type" and "tagger" lines
From: Johan Herland @ 2007-06-09  0:14 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Also update selftests to reflect that verification of "tagger" now
happens _before_ verification of type name, object sha1 and tag name.

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c          |   16 ++++++++--------
 t/t3800-mktag.sh |    3 +++
 tag.c            |    6 +++---
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/mktag.c b/mktag.c
index 0bc20c8..4dbefab 100644
--- a/mktag.c
+++ b/mktag.c
@@ -62,12 +62,18 @@ static int verify_tag(char *data, unsigned long size)
 
 	/* Verify tag-line */
 	tag_line = strchr(type_line, '\n');
-	if (!tag_line)
+	if (!tag_line++)
 		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - data);
-	tag_line++;
 	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
 		return error("char" PD_FMT ": no \"tag \" found", tag_line - data);
 
+	/* Verify the tagger line */
+	tagger_line = strchr(tag_line, '\n');
+	if (!tagger_line++)
+		return error("char" PD_FMT ": could not find next \"\\n\"", tag_line - data);
+	if (memcmp(tagger_line, "tagger ", 7) || (tagger_line[7] == '\n'))
+		return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - data);
+
 	/* Get the actual type */
 	type_len = tag_line - type_line - strlen("type \n");
 	if (type_len >= sizeof(type))
@@ -90,12 +96,6 @@ static int verify_tag(char *data, unsigned long size)
 		return error("char" PD_FMT ": could not verify tag name", tag_line - data);
 	}
 
-	/* Verify the tagger line */
-	tagger_line = tag_line;
-
-	if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n'))
-		return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - data);
-
 	/* TODO: check for committer info + blank line? */
 	/* Also, the minimum length is probably + "tagger .", or 63+8=71 */
 
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 7c7e433..b4edb4d 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -133,6 +133,7 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type taggggggggggggggggggggggggggggggg
 tag mytag
+tagger a
 EOF
 
 cat >expect.pat <<EOF
@@ -148,6 +149,7 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type tagggg
 tag mytag
+tagger a
 EOF
 
 cat >expect.pat <<EOF
@@ -163,6 +165,7 @@ cat >tag.sig <<EOF
 object $head
 type commit
 tag my	tag
+tagger a
 EOF
 
 cat >expect.pat <<EOF
diff --git a/tag.c b/tag.c
index 8d31603..19c66cd 100644
--- a/tag.c
+++ b/tag.c
@@ -73,10 +73,10 @@ static int parse_tag_buffer_internal(struct tag *item, const char *data, const u
 	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
 		return error("char" PD_FMT ": no \"tag \" found", tag_line - data);
 
+	/* Verify the tagger line */
 	tagger_line = strchr(tag_line, '\n');
-	if (!tagger_line)
-		return -1;
-	tagger_line++;
+	if (!tagger_line++)
+		return error("char" PD_FMT ": could not find next \"\\n\"", tag_line - data);
 
 	/* Get the actual type */
 	type_len = tag_line - type_line - strlen("type \n");
-- 
1.5.2

^ permalink raw reply related

* Re: fatal: serious inflate inconsistency
From: Thomas Glanzmann @ 2007-06-09  0:13 UTC (permalink / raw)
  To: Marco Roeland; +Cc: Linus Torvalds, Nicolas Pitre, GIT, Michael Gernoth
In-Reply-To: <20070608204557.GA18840@fiberbit.xs4all.nl>

Hello,
the OS is not corrupt. So I am going to compile a few kernels and wait
for gcc to segfault.

        Thomas

^ permalink raw reply

* [PATCH 03/21] Refactoring to make verify_tag() and parse_tag_buffer() more similar
From: Johan Herland @ 2007-06-09  0:13 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

A fair amount of refactoring is included in this patch, none of which
should affect the actual behaviour of the code in any way.

Here are the changes done:

- Refactor out all the (char *) casting in parse_tag_buffer(). Create a
  wrapper function (parse_tag_buffer()) that casts _once_ and then calls
  parse_tag_buffer_internal() which does the real work

- Variable renaming in parse_tag_buffer_internal():
  - sig_line -> tagger_line
  - typelen  -> type_len
  - taglen   -> tag_len

- Variable renaming in verify_tag():
  - buffer  -> data
  - typelen -> type_len

- Remove unnecessary variable 'object' from verify_tag(). It has always
  the same value as 'data', so use 'data' directly instead

- Fix type of length variables (int -> unsigned long)

- Move null-termination of tag buffer out of verify_tag() and into main().

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c |   41 +++++++++++++++++++----------------------
 tag.c   |   50 +++++++++++++++++++++++++++-----------------------
 2 files changed, 46 insertions(+), 45 deletions(-)

diff --git a/mktag.c b/mktag.c
index b82e377..0bc20c8 100644
--- a/mktag.c
+++ b/mktag.c
@@ -32,52 +32,48 @@ static int verify_object(unsigned char *sha1, const char *expected_type)
 	return ret;
 }
 
+static int verify_tag(char *data, unsigned long size)
+{
 #ifdef NO_C99_FORMAT
 #define PD_FMT "%d"
 #else
 #define PD_FMT "%td"
 #endif
 
-static int verify_tag(char *buffer, unsigned long size)
-{
-	int typelen;
-	char type[20];
 	unsigned char sha1[20];
-	const char *object, *type_line, *tag_line, *tagger_line;
+	char type[20];
+	const char *type_line, *tag_line, *tagger_line;
+	unsigned long type_len;
 
 	if (size < 64)
 		return error("wanna fool me ? you obviously got the size wrong !");
 
-	buffer[size] = 0;
-
 	/* Verify object line */
-	object = buffer;
-	if (memcmp(object, "object ", 7))
+	if (memcmp(data, "object ", 7))
 		return error("char%d: does not start with \"object \"", 0);
 
-	if (get_sha1_hex(object + 7, sha1))
+	if (get_sha1_hex(data + 7, sha1))
 		return error("char%d: could not get SHA1 hash", 7);
 
 	/* Verify type line */
-	type_line = object + 48;
+	type_line = data + 48;
 	if (memcmp(type_line - 1, "\ntype ", 6))
 		return error("char%d: could not find \"\\ntype \"", 47);
 
 	/* Verify tag-line */
 	tag_line = strchr(type_line, '\n');
 	if (!tag_line)
-		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - buffer);
+		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - data);
 	tag_line++;
 	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
-		return error("char" PD_FMT ": no \"tag \" found", tag_line - buffer);
+		return error("char" PD_FMT ": no \"tag \" found", tag_line - data);
 
 	/* Get the actual type */
-	typelen = tag_line - type_line - strlen("type \n");
-	if (typelen >= sizeof(type))
-		return error("char" PD_FMT ": type too long", type_line+5 - buffer);
-
-	memcpy(type, type_line+5, typelen);
-	type[typelen] = 0;
+	type_len = tag_line - type_line - strlen("type \n");
+	if (type_len >= sizeof(type))
+		return error("char" PD_FMT ": type too long", type_line + 5 - data);
+	memcpy(type, type_line + 5, type_len);
+	type[type_len] = '\0';
 
 	/* Verify that the object matches */
 	if (verify_object(sha1, type))
@@ -91,23 +87,23 @@ static int verify_tag(char *buffer, unsigned long size)
 			break;
 		if (c > ' ')
 			continue;
-		return error("char" PD_FMT ": could not verify tag name", tag_line - buffer);
+		return error("char" PD_FMT ": could not verify tag name", tag_line - data);
 	}
 
 	/* Verify the tagger line */
 	tagger_line = tag_line;
 
 	if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n'))
-		return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - buffer);
+		return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - data);
 
 	/* TODO: check for committer info + blank line? */
 	/* Also, the minimum length is probably + "tagger .", or 63+8=71 */
 
 	/* The actual stuff afterwards we don't care about.. */
 	return 0;
-}
 
 #undef PD_FMT
+}
 
 int main(int argc, char **argv)
 {
@@ -124,6 +120,7 @@ int main(int argc, char **argv)
 		free(buffer);
 		die("could not read from stdin");
 	}
+	buffer[size] = 0;
 
 	/* Verify it for some basic sanity: it needs to start with
 	   "object <sha1>\ntype\ntagger " */
diff --git a/tag.c b/tag.c
index 954ed8a..8d31603 100644
--- a/tag.c
+++ b/tag.c
@@ -33,7 +33,7 @@ struct tag *lookup_tag(const unsigned char *sha1)
         return (struct tag *) obj;
 }
 
-int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
+static int parse_tag_buffer_internal(struct tag *item, const char *data, const unsigned long size)
 {
 #ifdef NO_C99_FORMAT
 #define PD_FMT "%d"
@@ -41,14 +41,14 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 #define PD_FMT "%td"
 #endif
 
-	int typelen, taglen;
 	unsigned char sha1[20];
-	const char *type_line, *tag_line, *sig_line;
 	char type[20];
+	const char *type_line, *tag_line, *tagger_line;
+	unsigned long type_len, tag_len;
 
-        if (item->object.parsed)
-                return 0;
-        item->object.parsed = 1;
+	if (item->object.parsed)
+		return 0;
+	item->object.parsed = 1;
 
 	if (size < 64)
 		return error("failed preliminary size check");
@@ -57,39 +57,38 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 	if (memcmp(data, "object ", 7))
 		return error("char%d: does not start with \"object \"", 0);
 
-	if (get_sha1_hex((char *) data + 7, sha1))
+	if (get_sha1_hex(data + 7, sha1))
 		return error("char%d: could not get SHA1 hash", 7);
 
 	/* Verify type line */
-	type_line = (char *) data + 48;
+	type_line = data + 48;
 	if (memcmp(type_line - 1, "\ntype ", 6))
 		return error("char%d: could not find \"\\ntype \"", 47);
 
 	/* Verify tag-line */
 	tag_line = strchr(type_line, '\n');
 	if (!tag_line)
-		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - (char *) data);
+		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - data);
 	tag_line++;
 	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
-		return error("char" PD_FMT ": no \"tag \" found", tag_line - (char *) data);
+		return error("char" PD_FMT ": no \"tag \" found", tag_line - data);
 
-	sig_line = strchr(tag_line, '\n');
-	if (!sig_line)
+	tagger_line = strchr(tag_line, '\n');
+	if (!tagger_line)
 		return -1;
-	sig_line++;
+	tagger_line++;
 
 	/* Get the actual type */
-	typelen = tag_line - type_line - strlen("type \n");
-	if (typelen >= sizeof(type))
-		return error("char" PD_FMT ": type too long", type_line+5 - (char *) data);
-
-	memcpy(type, type_line + 5, typelen);
-	type[typelen] = '\0';
+	type_len = tag_line - type_line - strlen("type \n");
+	if (type_len >= sizeof(type))
+		return error("char" PD_FMT ": type too long", type_line + 5 - data);
+	memcpy(type, type_line + 5, type_len);
+	type[type_len] = '\0';
 
-	taglen = sig_line - tag_line - strlen("tag \n");
-	item->tag = xmalloc(taglen + 1);
-	memcpy(item->tag, tag_line + 4, taglen);
-	item->tag[taglen] = '\0';
+	tag_len = tagger_line - tag_line - strlen("tag \n");
+	item->tag = xmalloc(tag_len + 1);
+	memcpy(item->tag, tag_line + 4, tag_len);
+	item->tag[tag_len] = '\0';
 
 	if (!strcmp(type, blob_type)) {
 		item->tagged = &lookup_blob(sha1)->object;
@@ -115,6 +114,11 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 #undef PD_FMT
 }
 
+int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
+{
+	return parse_tag_buffer_internal(item, (const char *) data, size);
+}
+
 int parse_tag(struct tag *item)
 {
 	enum object_type type;
-- 
1.5.2

^ permalink raw reply related

* [PATCH 02/21] Return error messages when parsing fails.
From: Johan Herland @ 2007-06-09  0:13 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

This patch brings the already similar tag.c:parse_tag_buffer() and
mktag.c:verify_tag() a little bit closer to eachother.

Signed-off-by: Johan Herland <johan@herland.net>
---
 tag.c |   39 ++++++++++++++++++++++++++++++---------
 1 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/tag.c b/tag.c
index bbacd59..954ed8a 100644
--- a/tag.c
+++ b/tag.c
@@ -35,6 +35,12 @@ struct tag *lookup_tag(const unsigned char *sha1)
 
 int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 {
+#ifdef NO_C99_FORMAT
+#define PD_FMT "%d"
+#else
+#define PD_FMT "%td"
+#endif
+
 	int typelen, taglen;
 	unsigned char sha1[20];
 	const char *type_line, *tag_line, *sig_line;
@@ -45,28 +51,41 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
         item->object.parsed = 1;
 
 	if (size < 64)
-		return -1;
-	if (memcmp("object ", data, 7) || get_sha1_hex((char *) data + 7, sha1))
-		return -1;
+		return error("failed preliminary size check");
 
+	/* Verify object line */
+	if (memcmp(data, "object ", 7))
+		return error("char%d: does not start with \"object \"", 0);
+
+	if (get_sha1_hex((char *) data + 7, sha1))
+		return error("char%d: could not get SHA1 hash", 7);
+
+	/* Verify type line */
 	type_line = (char *) data + 48;
-	if (memcmp("\ntype ", type_line-1, 6))
-		return -1;
+	if (memcmp(type_line - 1, "\ntype ", 6))
+		return error("char%d: could not find \"\\ntype \"", 47);
 
+	/* Verify tag-line */
 	tag_line = strchr(type_line, '\n');
-	if (!tag_line || memcmp("tag ", ++tag_line, 4))
-		return -1;
+	if (!tag_line)
+		return error("char" PD_FMT ": could not find next \"\\n\"", type_line - (char *) data);
+	tag_line++;
+	if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
+		return error("char" PD_FMT ": no \"tag \" found", tag_line - (char *) data);
 
 	sig_line = strchr(tag_line, '\n');
 	if (!sig_line)
 		return -1;
 	sig_line++;
 
+	/* Get the actual type */
 	typelen = tag_line - type_line - strlen("type \n");
-	if (typelen >= 20)
-		return -1;
+	if (typelen >= sizeof(type))
+		return error("char" PD_FMT ": type too long", type_line+5 - (char *) data);
+
 	memcpy(type, type_line + 5, typelen);
 	type[typelen] = '\0';
+
 	taglen = sig_line - tag_line - strlen("tag \n");
 	item->tag = xmalloc(taglen + 1);
 	memcpy(item->tag, tag_line + 4, taglen);
@@ -92,6 +111,8 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
 	}
 
 	return 0;
+
+#undef PD_FMT
 }
 
 int parse_tag(struct tag *item)
-- 
1.5.2

^ permalink raw reply related

* [PATCH 01/21] Remove unnecessary code and comments on non-existing 8kB tag object restriction
From: Johan Herland @ 2007-06-09  0:12 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090210.36270.johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 mktag.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/mktag.c b/mktag.c
index 070bc96..b82e377 100644
--- a/mktag.c
+++ b/mktag.c
@@ -12,15 +12,8 @@
  * "object <sha1>\n" is 48 bytes, "type tag\n" at 9 bytes is the
  * shortest possible type-line, and "tag .\n" at 6 bytes is the
  * shortest single-character-tag line.
- *
- * We also artificially limit the size of the full object to 8kB.
- * Just because I'm a lazy bastard, and if you can't fit a signature
- * in that size, you're doing something wrong.
  */
 
-/* Some random size */
-#define MAXSIZE (8192)
-
 /*
  * We refuse to tag something we can't verify. Just because.
  */
-- 
1.5.2

^ permalink raw reply related

* [PATCH 0/21] Refactor the tag object (take 2)
From: Johan Herland @ 2007-06-09  0:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vzm3aig7j.fsf@assigned-by-dhcp.cox.net>

This patch series implements part of the ground work for the 'notes'
feature discussed earlier in the thread "[PATCH 00/15] git-note: A
mechanism for providing free-form after-the-fact annotations on commits".

The following patches refactors the tag object by:
1. Unifying parsing and verification of tag objects (patches 1-9)
2. Do better and more thorough verification of tag objects (patches 10-13)
3. Making the "tagger" header mandatory as far as possible (patch 14)
4. Making the "tag" header optional (patch 15)
5. Introducing a new optional "keywords" header (patch 16)
6. Auxiliary changes supporting the above (patches 17-21)

This patch series replaces the earlier patch series of the same name
(plus the current bugfixes on top of that series). It's also much easier
on the eyes for those with 80 chars wide displays. Also, the selftest
suite should run successfully at any point in this patch series.

Here's the shortlog:

Johan Herland (21):
      Remove unnecessary code and comments on non-existing 8kB tag object restriction
      Return error messages when parsing fails.
      Refactoring to make verify_tag() and parse_tag_buffer() more similar
      Refactor verification of "tagger" line to be more similar to verification of "type" and "tagger" lines
      Make parse_tag_buffer_internal() handle item == NULL
      Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line'
      Copy the remaining differences from verify_tag() to parse_tag_buffer_internal()
      Switch from verify_tag() to parse_and_verify_tag_buffer() for verifying tag objects in git-mktag
      Remove unneeded code from mktag.c
      Free mktag's buffer before dying
      Rewrite error messages; fix up line lengths
      Use prefixcmp() instead of memcmp() for cleaner code with less magic numbers
      Collect skipping of header field names and calculation of line lengths in one place
      Add proper parsing of "tagger" line, but only when thorough_verify is set
      Make tag names (i.e. the tag object's "tag" line) optional
      Introduce optional "keywords" on tag objects
      Update comments on tag objects in mktag.c
      git-fsck: Do thorough verification of tag objects
      Documentation/git-mktag: Document the changes in tag object structure
      git-mktag tests: Expand on mktag selftests according to the new tag object structure
      Add fsck_verify_ref_to_tag_object() to verify that refname matches name stored in tag object

 Documentation/git-mktag.txt |   41 +++++--
 builtin-fsck.c              |   41 +++++++
 builtin-log.c               |    2 +-
 mktag.c                     |  147 +++++-------------------
 t/t3800-mktag.sh            |  231 ++++++++++++++++++++++++++++++++++---
 tag.c                       |  268 +++++++++++++++++++++++++++++++++++--------
 tag.h                       |    5 +-
 7 files changed, 540 insertions(+), 195 deletions(-)


Have fun!

...Johan

^ permalink raw reply

* [RFC][PATCH 07/10] Sparse: fix "'merge_file' not declared" warning
From: Ramsay Jones @ 2007-06-08 22:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list


Add a declaration for merge_file() to a new "merge-file.h" header
file, and add the appropriate include directives.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

There are two other (static) definitions of a merge_file() function,
one in merge-recursive.c, which could probably be renamed merge_filespec()
and one in merge-index.c, which could be renamed merge_path().
Maybe the the function in merge-file.c should be called merge_blob() ...
(and then change the file names too!)

 Makefile     |    3 ++-
 merge-file.c |    1 +
 merge-file.h |    8 ++++++++
 merge-tree.c |    3 +--
 4 files changed, 12 insertions(+), 3 deletions(-)
 create mode 100644 merge-file.h

diff --git a/Makefile b/Makefile
index 29243c6..19b6da1 100644
--- a/Makefile
+++ b/Makefile
@@ -296,7 +296,8 @@ LIB_H = \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
-	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h mailmap.h
+	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
+	mailmap.h merge-file.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
diff --git a/merge-file.c b/merge-file.c
index 748d15c..7bfefe7 100644
--- a/merge-file.c
+++ b/merge-file.c
@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "xdiff-interface.h"
 #include "blob.h"
+#include "merge-file.h"
 
 static int fill_mmfile_blob(mmfile_t *f, struct blob *obj)
 {
diff --git a/merge-file.h b/merge-file.h
new file mode 100644
index 0000000..a503c90
--- /dev/null
+++ b/merge-file.h
@@ -0,0 +1,8 @@
+#ifndef MERGE_FILE_H
+#define MERGE_FILE_H
+
+#include "blob.h"
+
+extern void *merge_file(struct blob *, struct blob *, struct blob *, unsigned long *);
+
+#endif
diff --git a/merge-tree.c b/merge-tree.c
index 3b8d9e6..93c8f54 100644
--- a/merge-tree.c
+++ b/merge-tree.c
@@ -2,6 +2,7 @@
 #include "tree-walk.h"
 #include "xdiff-interface.h"
 #include "blob.h"
+#include "merge-file.h"
 
 static const char merge_tree_usage[] = "git-merge-tree <base-tree> <branch1> <branch2>";
 static int resolve_directories = 1;
@@ -53,8 +54,6 @@ static const char *explanation(struct merge_list *entry)
 	return "removed in remote";
 }
 
-extern void *merge_file(struct blob *, struct blob *, struct blob *, unsigned long *);
-
 static void *result(struct merge_list *entry, unsigned long *size)
 {
 	enum object_type type;
-- 
1.5.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox