Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX
Junio C Hamano venit, vidit, dixit 06.09.2017 05:35:
Show 20 quoted lines
> Michael J Gruber <git@grubix.eu> 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
>> constant.
>>
>> Change the remaining constant to the one appropriate for the current
>> type
>>
>> Signed-off-by: Michael J Gruber <git@grubix.eu>
>> ---
>
> Thanks.
>
> 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
for example.
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)...
Show 18 quoted lines
> 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,
Michael J Gruber
