[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Fix bogus code in libsvn_client/ra.c: slow_locations()

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2005-10-31 16:09:37 CET

In r15315 (r15581 for the 1.2.x branch), we made a change to improve the
performance of the location-tracing code against 1.0.x servers (which
lack the get_locations RA call); basically, we changed the implementation
to run 'log' only for the range that we cared about, youngest:oldest,
not youngest:1.

However, part of that change also simplified the code in slow_locations()
that works out which of the peg-revision, start-revision and end-revision
was the youngest (that is, highest in number), and in doing so
accidentally removed the only conditional assignment to the local
variable pegrev_is_youngest.

That made the section of code at the end of the function unconditional,
since pegrev_is_youngest was always FALSE.

  /* If our peg revision was smaller than either of our range
     revisions, we need to make sure that our calculated peg path is
     the same as what we expected it to be. */
  if (! pegrev_is_youngest)
    {
      if (strcmp (abs_path, lrb.peg_path) != 0)
        return svn_error_createf
          (SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
           _("'%s' in revision %ld is an unrelated object"),
           orig_path, youngest);
    }

Now, I don't fully understand this code, but I'm _fairly_ sure that
running it unconditionally is harmless, albeit unnecessary. However,
it's clearly bogus to depend upon a constant local variable, so the
attached patch reverts to the pre-r15315 behaviour of only checking the
peg path if the peg revision is older than the start or end revision.

[[[
* subversion/libsvn_client/ra.c
  (slow_locations): Remove useless local variable pegrev_is_youngest and
    revert to original (that is, pre-r15315) behaviour of checking the peg
    path only when the peg revision is not the youngest revision.
]]]

(I also changed 'smaller' to 'older' in the comment. because I found
the 'younger=larger revision number' concept hard enough to remember
without also having to handle both reference frames (younger/older,
smaller/larger) simultaneously.)

Regards,
Malcolm

Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 17107)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -601,7 +601,6 @@
   struct log_receiver_baton lrb = { 0 };
   apr_array_header_t *targets;
   svn_revnum_t youngest, oldest;
- svn_boolean_t pegrev_is_youngest = FALSE;
 
   /* Sanity check: verify that the peg-object exists in repos. */
   SVN_ERR (svn_ra_check_path (ra_session, "", peg_revnum, &(lrb.kind), pool));
@@ -658,10 +657,10 @@
        _("Unable to find repository location for '%s' in revision %ld"),
        orig_path, peg_revnum);
 
- /* If our peg revision was smaller than either of our range
+ /* If our peg revision was older than either of our range
      revisions, we need to make sure that our calculated peg path is
      the same as what we expected it to be. */
- if (! pegrev_is_youngest)
+ if (peg_revnum < youngest)
     {
       if (strcmp (abs_path, lrb.peg_path) != 0)
         return svn_error_createf

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Oct 31 16:10:32 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.