On Fri, Feb 22, 2013 at 03:20:10PM -0800, Junio C Hamano wrote:
> As pp_user_info() is called from very few places, I do not think it
> is unreasonable to add an output parameter (i.e. "unsigned *") to
> let the caller know that we made a best guess given malformed input
> and handle the error in the caller. The make_cover_letter() caller
> may look like:
>
> pp_user_info(&pp, NULL, &sb, committer, encoding, &errors);
> if (errors & PP_CORRUPT_DATE)
> warning("unparsable datestamp in '%s'", committer);
>
> although it is unlikely to see this error in practice, given that
> committer is coming from git_committer_info(0) and would have the
> current timestamp.
Sadly that is not quite enough for the object-parsing cases (which are
the ones we _really_ want to add context to, because they are buried
inside other pp_* calls. Probably adding an object context field (or an
error return) to the pretty-print context would make sense. But I don't
relish the thought of annotating each pretty-print caller.
I think we're OK to be silent and just react in an appropriate way;
having looked over the other callers of split_ident_line, we already do
so in some places. See my patch 1 below for details.
Once fsck is taught to note this, then the warning is a lot less
important (my patch 3 below).
> The whole "cat-file -p" is a historical wart, aka poor-man's
> "show". I do not even consider it a part of the plumbing. It is a
> fair game for Porcelainisque improvement ;-)
Good, that's how I feel, too. See my patch 4. :)
Here are the patches I'd like to do:
[1/4]: handle malformed dates in ident lines
[2/4]: skip_prefix: return a non-const pointer
[3/4]: fsck: check "tagger" lines
[4/4]: cat-file: print tags raw for "cat-file -p"
The first one is solid, and should probably go to maint and/or the -rc
track, as it fixes a segfault on bogus input. It's hopefully a
no-brainer, as the existing behavior is obviously unacceptable. We may
change our mind later about exactly what to print for such bogus input,
but whatever we print in such a case is just trying to be nice to the
user, and anybody who depends on our particular handling of malformed
objects is crazy.
The rest can wait, as they are about improving output when fed bogus
input, or tightening fsck. Moreover, they have some problems which make
them not suitable for applying yet. I'll give details in each patch.
-Peff
--
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