Re: [PATCH] Re: [Issue 816] - Silent failure in svn log -r

On 29 Jul 2002, Philip Martin wrote:

> > @@ -151,6 +153,51 @@
> > (&start_revnum, ra_lib, session, start, base_name, pool));
> > SVN_ERR (svn_client__get_revision_number
> > (&end_revnum, ra_lib, session, end, base_name, pool));
> > + SVN_ERR (svn_client__get_revision_number
> > + (&head_revnum, ra_lib, session, &head, base_name, pool));
>
> This is a round trip to the server. If start->kind or end->kind is
> head it can be avoided.
>
I hope I've fixed this now.
>
> Later in this function there is code that handles a new repository
> with no revisions:
> a) there is now code duplication retrieving HEAD.

Again, I hope I've fixed it.

> b) I think you broke it.
>
Yes I did.

Another patch attached below. Thanks for the feedback.

Regards

Kieran

-- 
Fix for issue 816
* subversion/libsvn_client/log.c (svn_client_log): Tests added to
ensure version numbers passed to it are between 0 and head inclusive
Throws SVN_ERR_CLIENT_BAD_REVISION if new tests fail.
* subversion/include/svn_client.h (svn_client_log comment text):
amended to mention the new revision number tests.
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c
+++ subversion/libsvn_client/log.c	Tue Jul 30 06:16:37 2002
@@ -62,9 +62,11 @@
   const char *URL;
   const char *base_name = NULL;
   apr_array_header_t *condensed_targets;
-  svn_revnum_t start_revnum, end_revnum;
+  svn_revnum_t start_revnum, end_revnum, youngest_rev;
   svn_error_t *err;
+  svn_client_revision_t head;
+  head.kind = svn_client_revision_head;
   if ((start->kind == svn_client_revision_unspecified)
       || (end->kind == svn_client_revision_unspecified))
     {
@@ -73,7 +75,7 @@
          "svn_client_log: caller failed to supply revision");
     }
-  start_revnum = end_revnum = SVN_INVALID_REVNUM;
+  start_revnum = end_revnum = youngest_rev = SVN_INVALID_REVNUM;
   path = (APR_ARRAY_IDX(targets, 0, const char *));
@@ -152,6 +154,83 @@
   SVN_ERR (svn_client__get_revision_number
            (&end_revnum, ra_lib, session, end, base_name, pool));
+  /* To save on network access, we check whether we already have head */
+  if (start->kind == svn_client_revision_head)
+    youngest_rev = start_revnum;
+  else if (end->kind == svn_client_revision_head)
+    youngest_rev = end_revnum;
+  else
+    SVN_ERR (svn_client__get_revision_number
+             (&youngest_rev, ra_lib, session, &head, base_name, pool));
+
+  /* If the repository has at least one version, we can check the bounds
+     strictly */
+  if (youngest_rev != 0 )
+    {
+      /* Here, we check that the versions input were valid.  If not, we
+         return an error */
+      if (start_revnum > youngest_rev && end_revnum > youngest_rev)
+        {
+          return svn_error_createf
+            (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
+             "Both revision numbers too high: %"
+             SVN_REVNUM_T_FMT " and %" SVN_REVNUM_T_FMT
+             ".  Head is %" SVN_REVNUM_T_FMT ".",
+             start_revnum, end_revnum, youngest_rev);
+        }
+
+      if (start_revnum > youngest_rev || end_revnum > youngest_rev)
+        {
+          return svn_error_createf
+            (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
+             "Revision number too high: %" SVN_REVNUM_T_FMT
+             ".  Head is %" SVN_REVNUM_T_FMT ".",
+             (start_revnum > end_revnum) ? start_revnum : end_revnum,
+             youngest_rev);
+        }
+    }
+  else
+    {
+      /* Special case of an empty repository.  See comment below for details */
+      if (start_revnum > 1 && end_revnum > 1)
+        {
+          return svn_error_createf
+            (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
+             "Both revision numbers too high: %"
+             SVN_REVNUM_T_FMT " and %" SVN_REVNUM_T_FMT
+             ".", start_revnum, end_revnum);
+        }
+
+      if (start_revnum > youngest_rev + 1|| end_revnum > youngest_rev + 1)
+        {
+          return svn_error_createf
+            (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
+             "Revision number too high: %" SVN_REVNUM_T_FMT
+             ".", (start_revnum > end_revnum) ? start_revnum : end_revnum);
+        }
+
+
+    }
+  if (start_revnum < 0 && end_revnum < 0)
+    {
+      return svn_error_createf
+        (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
+         "Both revision numbers are negative: %"
+         SVN_REVNUM_T_FMT " and %" SVN_REVNUM_T_FMT
+         ".  Negative revision number are not allowed.",
+         start_revnum, end_revnum);
+
+    }
+
+  if (start_revnum < 0 || end_revnum < 0)
+    {
+      return svn_error_createf
+        (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
+         "Revision number negative: %" SVN_REVNUM_T_FMT
+         ".  Negative revision numbers are not allowed.",
+         (start_revnum < end_revnum) ? start_revnum : end_revnum);
+    }
+
   err = ra_lib->get_log (session,
                          condensed_targets,
                          start_revnum,
@@ -179,9 +258,7 @@
       && ((end->kind == svn_client_revision_number)
           && (end->value.number == 1)))
     {
-      svn_revnum_t youngest_rev;
-
-      SVN_ERR (ra_lib->get_latest_revnum (session, &youngest_rev));
+
       if (youngest_rev == 0)
         {
           err = SVN_NO_ERROR;
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h
+++ subversion/include/svn_client.h	Mon Jul 29 01:24:52 2002
@@ -557,6 +557,13 @@
    If START->kind or END->kind is svn_client_revision_unspecified,
    return the error SVN_ERR_CLIENT_BAD_REVISION.
+   If the version number returned after canonicalising either START or
+   FINISH is greater than that of head, return the error
+   SVN_ERR_CLIENT_BAD_REVISION
+
+   If the version number returned after canonicalising either START or
+   FINISH is negative, return the error SVN_ERR_CLIENT_BAD_REVISION
+
    Use POOL for any temporary allocation.
    Special case for repositories at revision 0:
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org