From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Mantas Mikulėnas" <grawity@gmail.com>, git@vger.kernel.org
Subject: Re: Crashes while trying to show tag objects with bad timestamps
Date: Fri, 22 Feb 2013 15:20:10 -0800 [thread overview]
Message-ID: <7vppzsaqc5.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20130222230418.GC21579@sigill.intra.peff.net> (Jeff King's message of "Fri, 22 Feb 2013 18:04:18 -0500")
Jeff King <peff@peff.net> writes:
> On Fri, Feb 22, 2013 at 02:53:48PM -0800, Junio C Hamano wrote:
>
>> > I guess we should probably issue a warning, too. Also disappointingly,
>> > git-fsck does not seem to detect this breakage at all.
>>
>> Yes for the warning,
>
> Unfortunately, a good warning is harder than I had hoped. At the point
> where we notice the problem, pp_user_info, we have very little context.
> We can say only something like:
>
> warning: malformed date in ident 'Jeff King <peff@peff.net> BOGUS'
>
> but we cannot say in which object, or even that it was a "tagger" line
> (and in some cases we do not even have an object, as in
> make_cover_letter).
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.
> I also took a look at parsing routine of "cat-file -p". It's totally
> hand-rolled, separate from what "git show" does, and is not build on the
> pretty-print code at all. I wonder, though, if it actually makes sense
> to munge the date there. The commit-object pretty-printer for cat-file
> just shows the object intact. It seems weirdly inconsistent that we
> would munge tags just to rewrite the date. If you want a real
> pretty-printer, you should be using porcelain like "show".
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 ;-)
next prev parent reply other threads:[~2013-02-22 23:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-22 22:30 Crashes while trying to show tag objects with bad timestamps Mantas Mikulėnas
2013-02-22 22:46 ` Jeff King
2013-02-22 22:53 ` Junio C Hamano
2013-02-22 23:04 ` Jeff King
2013-02-22 23:14 ` Mantas Mikulėnas
2013-02-25 18:21 ` Jeff King
2013-02-22 23:20 ` Junio C Hamano [this message]
2013-02-25 18:30 ` Jeff King
2013-02-25 18:38 ` [PATCH 1/4] handle malformed dates in ident lines Jeff King
2013-02-25 18:39 ` [PATCH/RFC 2/4] skip_prefix: return a non-const pointer Jeff King
2013-02-25 18:46 ` [PATCH 3/4] fsck: check "tagger" lines Jeff King
2013-02-25 18:50 ` [PATCH 4/4] cat-file: print tags raw for "cat-file -p" Jeff King
2013-02-25 19:33 ` Mantas Mikulėnas
2013-02-22 23:01 ` [RFC/PATCH] hash-object doc: "git hash-object -w" can write invalid objects Jonathan Nieder
2013-02-22 23:07 ` Junio C Hamano
2013-02-22 23:09 ` Jeff King
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vppzsaqc5.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=grawity@gmail.com \
--cc=peff@peff.net \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).