[PATCH 2/2] added more informative error messages to git-mktag

[PATCH 2/2] added more informative error messages to git-mktag

[Björn Engelmann]

Signed-off-by: Björn Engelmann <BjEngelmann@gmx.de>


---

5045fd2336abbe57f191e8839759369ee1c7cc6e
 mktag.c |   41 +++++++++++++++++++++++++++++++----------
 1 files changed, 31 insertions(+), 10 deletions(-)

5045fd2336abbe57f191e8839759369ee1c7cc6e
diff --git a/mktag.c b/mktag.c
index 79c466c..9f4878c 100644
--- a/mktag.c
+++ b/mktag.c
@@ -45,42 +45,60 @@ static int verify_tag(char *buffer, unsi
     unsigned char sha1[20];
     const char *object, *type_line, *tag_line, *tagger_line;
 
-    if (size < 64)
+    if (size < 64) {
+        printf("wanna fool me ? you obviously got the size wrong !\n");
         return -1;
+    }
     buffer[size] = 0;
 
     /* Verify object line */
     object = buffer;
-    if (memcmp(object, "object ", 7))
+    if (memcmp(object, "object ", 7)) {
+        printf("char%i: does not start with \"object \"\n", 0);
         return -1;
-    if (get_sha1_hex(object + 7, sha1))
+    }
+    if (get_sha1_hex(object + 7, sha1)) {
+        printf("char%i: could not get SHA1 hash\n", 7);
         return -1;
+    }
 
     /* Verify type line */
     type_line = object + 48;
-    if (memcmp(type_line - 1, "\ntype ", 6))
+    if (memcmp(type_line - 1, "\ntype ", 6)) {
+        printf("char%i: could not find \"\\ntype \"\n", 47);
         return -1;
+    }
 
     /* Verify tag-line */
     tag_line = strchr(type_line, '\n');
-    if (!tag_line)
+    if (!tag_line) {
+        printf("char%i: could not find next \"\\n\"\n", (int)type_line
- (int)buffer);
         return -1;
+    }
     tag_line++;
-    if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
+    if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n') {
+        printf("char%i: no \"tag \" found\n", (int)tag_line - (int)buffer);
         return -1;
+    }
 
     /* Get the actual type */
     typelen = tag_line - type_line - strlen("type \n");
-    if (typelen >= sizeof(type))
+    if (typelen >= sizeof(type)) {
+        printf("char%i: type too long\n", (int)type_line + strlen("type
\n") - (int)buffer);
         return -1;
+    }
     memcpy(type, type_line+5, typelen);
     type[typelen] = 0;
 
     /* Verify that the object matches */
-    if (get_sha1_hex(object + 7, sha1))
+    if (get_sha1_hex(object + 7, sha1)) {
+        printf("char%i: could not get SHA1 hash but this is really odd
since i got it before !\n", 7);
         return -1;
-    if (verify_object(sha1, type))
+    }
+    if (verify_object(sha1, type)) {
+        printf("char%i: could not verify object %s\n", 7, sha1);
         return -1;
+    }
 
     /* Verify the tag-name: we don't allow control characters or spaces
in it */
     tag_line += 4;
@@ -90,14 +108,17 @@ static int verify_tag(char *buffer, unsi
             break;
         if (c > ' ')
             continue;
+        printf("char%i: could not verify tag name\n", (int)tag_line -
(int)buffer);
         return -1;
     }
 
     /* Verify the tagger line */
     tagger_line = tag_line;
 
-    if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n'))
+    if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n')) {
+        printf("char%i: could not find \"tagger\"\n", (int)tagger_line
- (int)buffer);
         return -1;
+    }
 
     /* The actual stuff afterwards we don't care about.. */
     return 0;
-- 
1.3.3.g5045-dirty

Re: [PATCH 2/2] added more informative error messages to git-mktag

[Junio C Hamano]

Björn Engelmann <BjEngelmann@gmx.de> writes:

> -    if (size < 64)
> +    if (size < 64) {
> +        printf("wanna fool me ? you obviously got the size wrong !\n");
>          return -1;
> +    }

Please do this instead:

	return error("wanna ...");

you can lose the braces and the message goes to the stderr.

> -    if (memcmp(object, "object ", 7))
> +    if (memcmp(object, "object ", 7)) {
> +        printf("char%i: does not start with \"object \"\n", 0);
>          return -1;

Although they may be synonyms, we tend to use %d for ints and it
is more conventional.

>      tag_line++;
> -    if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
> +    if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n') {
> +        printf("char%i: no \"tag \" found\n", (int)tag_line - (int)buffer);
>          return -1;
> +    }

If you have to cast, please do not cast pointers to ints and
take their difference, but take the difference and cast the
resulting ptrdiff_t to int, like this:

	(int)(tag_line - buffer)

Or use "%td" instead of "%i" and lose the cast.