git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: [PATCH] Silence error messages unless 'thorough_verify' is set
Date: Sat, 09 Jun 2007 21:42:05 +0200	[thread overview]
Message-ID: <200706092142.05446.johan@herland.net> (raw)
In-Reply-To: <200706092028.54459.johan@herland.net>

We don't want to print error message on regular parsing of tag objects.

With this patch error messages are only printed when 'thorough_verify'
is set, i.e. when creating new or fscking tag objects.

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---

On Saturday 09 June 2007, Johan Herland wrote:
> On Saturday 09 June 2007, Junio C Hamano wrote:
> > While I would agree that it makes sense to have the same
> > definition of what is and is not a 100% well formatted tag
> > object for producer side and consumer side, I do not necessarily
> > think it is a good idea to make parse_tag_buffer() chattier on
> > errors.  mktag.c:verify_tag() can afford to be verbose in its
> > diagnosis, because it is used when the user is _creating_ the
> > tag, and it is generally a good idea to be strict when we
> > create.
> > 
> > On the other hand, parse_tag_buffer() is on the side of users
> > who use existing tag objects that were produced by somebody
> > else.  It is generally a good practice to be more lenient when
> > you are consuming.
> > 
> > Also, callers of parse_tag_buffer() know the function is silent
> > on errors (unless there is something seriously wrong with the
> > repository); they do their own error messages when they get an
> > error return.
> 
> Ok. I can make the error messages conditional on 'thorough_verify'.

Is this one ok?

(It goes on top of the patch series as a whole)


...Johan

 tag.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/tag.c b/tag.c
index af4356e..c3a2855 100644
--- a/tag.c
+++ b/tag.c
@@ -67,6 +67,8 @@ int parse_and_verify_tag_buffer(struct tag *item,
 #define PD_FMT "%td"
 #endif
 
+#define FAIL(...) ( thorough_verify ? error(__VA_ARGS__) : -1 )
+
 	unsigned char sha1[20];
 	char type[20];
 	const char   *type_line, *tag_line, *keywords_line, *tagger_line;
@@ -80,26 +82,26 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	}
 
 	if (size < 65)
-		return error("Tag object failed preliminary size check");
+		return FAIL("Tag object failed preliminary size check");
 
 	/* Verify mandatory object line */
 	if (prefixcmp(data, "object "))
-		return error("Tag object (@ char 0): "
+		return FAIL("Tag object (@ char 0): "
 			"Does not start with \"object \"");
 
 	if (get_sha1_hex(data + 7, sha1))
-		return error("Tag object (@ char 7): Could not get SHA1 hash");
+		return FAIL("Tag object (@ char 7): Could not get SHA1 hash");
 
 	/* Verify mandatory type line */
 	type_line = data + 48;
 	if (prefixcmp(type_line - 1, "\ntype "))
-		return error("Tag object (@ char 47): "
+		return FAIL("Tag object (@ char 47): "
 			"Could not find \"\\ntype \"");
 
 	/* Verify optional tag line */
 	tag_line = strchr(type_line, '\n');
 	if (!tag_line++)
-		return error("Tag object (@ char " PD_FMT "): "
+		return FAIL("Tag object (@ char " PD_FMT "): "
 			"Could not find \"\\n\" after \"type\"",
 			type_line - data);
 	if (prefixcmp(tag_line, "tag ")) /* no tag name given */
@@ -107,7 +109,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	else {                           /* tag name given */
 		keywords_line = strchr(tag_line, '\n');
 		if (!keywords_line++)
-			return error("Tag object (@ char " PD_FMT "): "
+			return FAIL("Tag object (@ char " PD_FMT "): "
 				"Could not find \"\\n\" after \"tag\"",
 				tag_line - data);
 	}
@@ -118,7 +120,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	else {                                     /* keywords given */
 		tagger_line = strchr(keywords_line, '\n');
 		if (!tagger_line++)
-			return error("Tag object (@ char " PD_FMT "): "
+			return FAIL("Tag object (@ char " PD_FMT "): "
 				"Could not find \"\\n\" after \"keywords\"",
 				keywords_line - data);
 	}
@@ -132,7 +134,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	 */
 	if (prefixcmp(tagger_line, "tagger ")) { /* no tagger given */
 		if (thorough_verify)
-			return error("Tag object (@ char " PD_FMT "): "
+			return FAIL("Tag object (@ char " PD_FMT "): "
 				"Could not find \"tagger \"",
 				tagger_line - data);
 		header_end = tagger_line;
@@ -140,7 +142,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 	else {                                   /* tagger given */
 		header_end = strchr(tagger_line, '\n');
 		if (!header_end++)
-			return error("Tag object (@ char " PD_FMT "): "
+			return FAIL("Tag object (@ char " PD_FMT "): "
 				"Could not find \"\\n\" after \"tagger\"",
 				tagger_line - data);
 	}
@@ -164,7 +166,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 
 	/* Get the actual type */
 	if (type_len >= sizeof(type))
-		return error("Tag object (@ char " PD_FMT "): "
+		return FAIL("Tag object (@ char " PD_FMT "): "
 			"Type too long", type_line - data);
 	memcpy(type, type_line, type_len);
 	type[type_len] = '\0';
@@ -174,7 +176,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 
 		/* Verify that the object matches */
 		if (verify_object(sha1, type))
-			return error("Tag object (@ char 7): "
+			return FAIL("Tag object (@ char 7): "
 				"Could not verify tagged object %s",
 				sha1_to_hex(sha1));
 
@@ -184,7 +186,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 				unsigned char c = tag_line[i];
 				if (c > ' ' && c != 0x7f)
 					continue;
-				return error("Tag object (@ char " PD_FMT "): "
+				return FAIL("Tag object (@ char " PD_FMT "): "
 					"Could not verify tag name",
 					tag_line + i - data);
 			}
@@ -199,12 +201,12 @@ int parse_and_verify_tag_buffer(struct tag *item,
 				unsigned char c = keywords_line[i];
 				if (c == ',' && keywords_line[i + 1] == ',')
 					/* consecutive commas */
-					return error("Tag object (@ char "
+					return FAIL("Tag object (@ char "
 						PD_FMT "): Found empty keyword",
 						keywords_line + i - data);
 				if (c > ' ' && c != 0x7f)
 					continue;
-				return error("Tag object (@ char " PD_FMT "): "
+				return FAIL("Tag object (@ char " PD_FMT "): "
 					"Could not verify keywords",
 					keywords_line + i - data);
 			}
@@ -248,7 +250,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
 		} else if (!strcmp(type, tag_type)) {
 			item->tagged = &lookup_tag(sha1)->object;
 		} else {
-			error("Tag object (@ char " PD_FMT "): "
+			FAIL("Tag object (@ char " PD_FMT "): "
 				"Unknown type '%s'", type_line - data, type);
 			item->tagged = NULL;
 		}
@@ -262,6 +264,8 @@ int parse_and_verify_tag_buffer(struct tag *item,
 
 	return 0;
 
+#undef FAIL
+
 #undef PD_FMT
 }
 
-- 
1.5.2.1.144.gabc40

  reply	other threads:[~2007-06-09 19:42 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-07 22:50 error: char103: premature end of data Johannes Schindelin
2007-06-07 23:05 ` Johan Herland
2007-06-07 23:28   ` Johannes Schindelin
2007-06-07 23:47     ` Johan Herland
2007-06-07 23:55       ` Johannes Schindelin
2007-06-08  0:08       ` [PATCH] Fix failed tag parsing when tag object has no body/message (and thus ends with a single '\n') Johan Herland
2007-06-08  6:05         ` Junio C Hamano
2007-06-08  8:18           ` Johan Herland
2007-06-08 16:06             ` Junio C Hamano
2007-06-09  0:10               ` [PATCH 0/21] Refactor the tag object (take 2) Johan Herland
2007-06-09  0:12                 ` [PATCH 01/21] Remove unnecessary code and comments on non-existing 8kB tag object restriction Johan Herland
2007-06-09  0:13                 ` [PATCH 02/21] Return error messages when parsing fails Johan Herland
2007-06-09 18:01                   ` Junio C Hamano
2007-06-09 18:28                     ` Johan Herland
2007-06-09 19:42                       ` Johan Herland [this message]
2007-06-10  6:48                         ` [PATCH] Silence error messages unless 'thorough_verify' is set Johannes Schindelin
2007-06-10  8:15                           ` Junio C Hamano
2007-06-10 10:08                             ` Johannes Schindelin
2007-06-10 12:10                               ` Johan Herland
2007-06-10 18:51                                 ` Johannes Schindelin
2007-06-10 19:16                                   ` Johan Herland
2007-06-10 11:47                             ` [PATCH 0/4] Restructure the tag object Johan Herland
2007-06-10 11:49                               ` [PATCH 1/4] Make tag names (i.e. the tag object's "tag" line) optional Johan Herland
2007-06-10 22:46                                 ` Junio C Hamano
2007-06-10 23:01                                   ` Johan Herland
2007-06-11  1:11                                     ` Junio C Hamano
2007-06-10 11:50                               ` [PATCH 2/4] Introduce optional "keywords" on tag objects Johan Herland
2007-06-10 18:42                                 ` Johannes Schindelin
2007-06-10 19:04                                   ` Johan Herland
2007-06-10 21:43                                     ` Junio C Hamano
2007-06-10 23:16                                       ` Johan Herland
2007-06-11  1:01                                         ` Junio C Hamano
2007-06-10 11:50                               ` [PATCH 3/4] Documentation/git-mktag: Document the changes in tag object structure Johan Herland
2007-06-10 11:50                               ` [PATCH 4/4] git-mktag tests: Expand on mktag selftests according to the new " Johan Herland
2007-06-10 18:35                               ` [PATCH 0/4] Restructure the tag object Johannes Schindelin
2007-06-09  0:13                 ` [PATCH 03/21] Refactoring to make verify_tag() and parse_tag_buffer() more similar Johan Herland
2007-06-09  2:54                   ` Johannes Schindelin
2007-06-09 10:49                     ` Johan Herland
2007-06-09  0:14                 ` [PATCH 04/21] Refactor verification of "tagger" line to be more similar to verification of "type" and "tagger" lines Johan Herland
2007-06-09 18:01                   ` Junio C Hamano
2007-06-10  7:49                     ` Johannes Schindelin
2007-06-09  0:14                 ` [PATCH 05/21] Make parse_tag_buffer_internal() handle item == NULL Johan Herland
2007-06-09 18:01                   ` Junio C Hamano
2007-06-10  0:45                     ` [PATCH] Move check for already parsed tag object to parse_tag_buffer() wrapper function Johan Herland
2007-06-10  8:06                   ` [PATCH 05/21] Make parse_tag_buffer_internal() handle item == NULL Johannes Schindelin
2007-06-09  0:15                 ` [PATCH 06/21] Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line' Johan Herland
2007-06-09 21:26                   ` Alex Riesen
2007-06-09 21:34                     ` Johan Herland
2007-06-10  8:14                       ` Johannes Schindelin
2007-06-10  9:01                   ` Johannes Schindelin
2007-06-09  0:15                 ` [PATCH 07/21] Copy the remaining differences from verify_tag() to parse_tag_buffer_internal() Johan Herland
2007-06-09 21:31                   ` Alex Riesen
2007-06-09 21:39                     ` Johan Herland
2007-06-10  8:22                       ` Johannes Schindelin
2007-06-09  0:15                 ` [PATCH 08/21] Switch from verify_tag() to parse_and_verify_tag_buffer() for verifying tag objects in git-mktag Johan Herland
2007-06-09  0:16                 ` [PATCH 09/21] Remove unneeded code from mktag.c Johan Herland
2007-06-09 21:39                   ` Alex Riesen
2007-06-09 21:42                     ` Johan Herland
2007-06-09  0:16                 ` [PATCH 10/21] Free mktag's buffer before dying Johan Herland
2007-06-09 21:37                   ` Alex Riesen
2007-06-09 21:46                     ` Johan Herland
2007-06-09 22:00                       ` Alex Riesen
2007-06-09 22:05                         ` Johan Herland
2007-06-10  8:38                   ` Johannes Schindelin
2007-06-09  0:17                 ` [PATCH 11/21] Rewrite error messages; fix up line lengths Johan Herland
2007-06-10  8:38                   ` Johannes Schindelin
2007-06-09  0:17                 ` [PATCH 12/21] Use prefixcmp() instead of memcmp() for cleaner code with less magic numbers Johan Herland
2007-06-09 21:42                   ` Alex Riesen
2007-06-09 21:47                     ` Johan Herland
2007-06-10  8:41                   ` Johannes Schindelin
2007-06-09  0:18                 ` [PATCH 13/21] Collect skipping of header field names and calculation of line lengths in one place Johan Herland
2007-06-10  8:45                   ` Johannes Schindelin
2007-06-09  0:18                 ` [PATCH 14/21] Add proper parsing of "tagger" line, but only when thorough_verify is set Johan Herland
2007-06-10  8:52                   ` Johannes Schindelin
2007-06-10  8:58                   ` Johannes Schindelin
2007-06-09  0:19                 ` [PATCH 15/21] Make tag names (i.e. the tag object's "tag" line) optional Johan Herland
2007-06-10  9:07                   ` Johannes Schindelin
2007-06-09  0:19                 ` [PATCH 16/21] Introduce optional "keywords" on tag objects Johan Herland
2007-06-09 21:52                   ` Alex Riesen
2007-06-09 22:00                     ` Johan Herland
2007-06-09 22:36                     ` [PATCH] Use xstrndup() instead of xmalloc() and memcpy(); fix buglet with generating default item->keywords Johan Herland
2007-06-10  0:05                     ` [PATCH 16/21] Introduce optional "keywords" on tag objects Junio C Hamano
2007-06-10  0:35                       ` [PATCH] Fail if tag name and keywords is not within "printable ASCII" Johan Herland
2007-06-10  1:33                         ` Junio C Hamano
2007-06-09  0:20                 ` [PATCH 17/21] Update comments on tag objects in mktag.c Johan Herland
2007-06-09  0:20                 ` [PATCH 18/21] git-fsck: Do thorough verification of tag objects Johan Herland
2007-06-09  0:20                 ` [PATCH 19/21] Documentation/git-mktag: Document the changes in tag object structure Johan Herland
2007-06-09  0:21                 ` [PATCH 20/21] git-mktag tests: Expand on mktag selftests according to the new " Johan Herland
2007-06-09  0:21                 ` [PATCH 21/21] Add fsck_verify_ref_to_tag_object() to verify that refname matches name stored in tag object Johan Herland
2007-06-07 23:11 ` error: char103: premature end of data Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200706092142.05446.johan@herland.net \
    --to=johan@herland.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).