Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX
- Date: Wed, 6 Sep 2017 13:59:31 +0200
- From: Michael J Gruber <michael@xxxxxxxxx>
- Subject: Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX
Junio C Hamano venit, vidit, dixit 06.09.2017 05:35:
> Michael J Gruber <git@xxxxxxxxx> writes:
>> Earlier, dddbad728c ("timestamp_t: a new data type for timestamps",
>> 2017-04-26) changed several types to timestamp_t.
>> 5589e87fd8 ("name-rev: change a "long" variable to timestamp_t",
>> 2017-05-20) cleaned up a missed variable, but both missed a _MAX
>> Change the remaining constant to the one appropriate for the current
>> Signed-off-by: Michael J Gruber <git@xxxxxxxxx>
> I think this (and the earlier 5589e8) was caused by an unnoticed
> semantic conflict at 78089b71 ("Merge branch 'jc/name-rev-lw-tag'",
> 2017-05-30). Merging is sometimes hard ;-)
Simple merges and semi-simple merges...
BTW, there's more fallout from those name-rev changes: In connection
with that other thread about surprising describe results for emacs.git I
noticed that I can easily get "git name-rev --stdin" to segfault there.
As easy as
echo bc5d96a0b2a1dccf7eeeec459e40d21b54c977f4 | git name-rev --stdin
That's unfortunate for the use-case of name-rev to amend git log output.
The reason seems to be that with "--stdin" or "--all", "name-rev" walks
and names all commits before beginning to use that those names for even
a single commit as above.
That segfault bisects to the logic changing commit in
jc/name-rev-lw-tag, but I think the changed logic simply leads to more
xmallocs() the segfault sooner now. Or something that I dind't spot even
after a few hours.
On the other hand, nearly every time that I try to understand describe
or name-rev I want get rid of insert_commit_by_date() and the like and
replace this by generations, and maybe a simple rev-walk (per ref)...
> Will queue.
>> builtin/name-rev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
>> index c41ea7c2a6..598da6c8bc 100644
>> --- a/builtin/name-rev.c
>> +++ b/builtin/name-rev.c
>> @@ -253,7 +253,7 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
>> struct commit *commit = (struct commit *)o;
>> int from_tag = starts_with(path, "refs/tags/");
>> - if (taggerdate == ULONG_MAX)
>> + if (taggerdate == TIME_MAX)
>> taggerdate = ((struct commit *)o)->date;
>> path = name_ref_abbrev(path, can_abbreviate_output);
>> name_rev(commit, xstrdup(path), taggerdate, 0, 0,