Re: Issue #4467 - blame youngest to oldest needs to handle SVN_INVALID_REVNUM - 1.9 blocker?

Julian Foad <julianfoad_at_btopenworld.com> writes:

> This issue looks like one that we should fix for 1.9.
>
> [[[
> Several of the blame functions don't work properly when start=HEAD and end=N
> because they compare start and end to work out whether start is less than or
> greater than end.  At least these functions need attention:
>
>   svn_client_blame5 gets younger_rev wrong
>   svn_ra_get_file_revs2 gets the capability check wrong
>   svn_ra_serf__get_file_revs gets the peg_rev wrong
>
> See also r1568872 which fixed a similar problem with log.
> ]]]
>
> Philip wrote on IRC just now,
>
> [[[
>
> $ svn blame -rHEAD:1000000 ../src/README
> svn: E235000: In file '../src/subversion/libsvn_client/blame.c' line 497: assertion failed ((frb->last_filename == NULL) || frb->include_merged_revisions)
>
> [...] I think it is a consequence of things like blame.c:659 which does younger_end.value.number = MAX(start_revnum, end_revnum);
> ]]]
>
> Anyone want to take a look at it?

r1568872 was

Index: ../src/subversion/libsvn_ra_serf/log.c
===================================================================
--- ../src/subversion/libsvn_ra_serf/log.c (revision 1568871)
+++ ../src/subversion/libsvn_ra_serf/log.c (revision 1568872)
@@ -602,7 +602,7 @@
   /* At this point, we may have a deleted file. So, we'll match ra_neon's
    * behavior and use the larger of start or end as our 'peg' rev.
    */
- peg_rev = (start > end) ? start : end;
+ peg_rev = (start == SVN_INVALID_REVNUM || start > end) ? start : end;
 
   SVN_ERR(svn_ra_serf__get_stable_url(&req_url, NULL /* latest_revnum */,
                                       session, NULL /* conn */,

Essentially every time we compare start and end we need to make sure
that we handle SVN_INVALID_REVNUM properly. I think the functions
listed in the issue are a few locations I identified that need to be
checked.

The issue identifies svn_client_blame5 but looking at the code it
converts start and end to numerical revisions early on so probably does
not have the bug. The SEGV above may be some other problem.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*