Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> +#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");
This is ugly.
If you _have_ to output the error message in one case, and not in the
other, I'd rather do
enum tag_error { TAG_SIZE_CHECK, TAG_BLA_BLUB, ... };
const char **tag_error_strings = { "tag: size error", ... };
Of course, you'd lose the ability to output some numbers. But those
numbers that you output are even uglier than the code. Guess how surprised
_I_ was, when I hit the error message which made me go mad.
Having said that, I still do not agree in this unifying.
Your rationale seems to be: use the same checking for the tag creation as
for the tag validation.
But this is _wrong_. We _do_ have tags that do not conform to the strict
standards of git-tag, and even if we did _not_, it would _still_ be wrong
to be that strict when _reading_ tags.
To drive that point home: strict checking when creating tags is good.
Strict checking when reading tags is bad.
I strongly encourage keeping both validations separate.
You'd also avoid having that many lines which are well over the encouraged
80 character limit.
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
