Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> On Sat, 9 Jun 2007, Johan Herland wrote:
> ...
>> @@ -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.
... quite a bit. A less uglier alternative we seem to use in
other places is not much better (return NULL on failure or an
error message string on error).
> ... Guess how surprised
> _I_ was, when I hit the error message which made me go mad.
To be fair, that ugly "char%d" was taken from mktag and not
Johan's invention.
> 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.
While I tend ot think that keeping two separate versions is
probably better for this particular case, the above statement
has a leap in its logic. With your "error code" scheme, you
could implement a single, verifier/parser that defines the
concrete and complete rule of how the data should look like.
That unified verifier/parser itself should be silent. Then, you
can have each of the callers decide how lenient it wants to be,
depending on the seriousness of the error. You can make
producer very strict and chatty while leaving consumer liberal
and more silent.
There are pros-and-cons, however.
- Such a scheme to return error codes and have two callers that
have different behaviours is cumbersome to set up and use.
A good example of this is the switch/case mess in each of the
callers of run_command_v_opt() in builtin-push.c,
builtin-revert.c, receive-pack.c etc. For run_command, the
mess is justifiable because the function has enough number of
different callers, but in the current thread, we are only
talking about two callers (parsing vs verifying of tag
objects).
- It has a risk to introduce inconsitent definition of the data
format to have completely separate producer and consumer
implementations; this is especially true when the data in
question is complex.
However, a tag is sufficiently simple that my personal
feeling is that, combined with the cumbersomeness argument
against the unified verifier, separate producer and consumer
implementations would be easier to manage for this particular
case.
-
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
