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

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

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2002-07-29 20:27:05 CEST

Kieran <kieran@esperi.demon.co.uk> writes:

> 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 Sat Jul 27 18:52:04 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, head_revnum;
> 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 = head_revnum = SVN_INVALID_REVNUM;
>
> path = (APR_ARRAY_IDX(targets, 0, const char *));
>
> @@ -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.

> +
> + /* Here, we check that the versions input were valid. If not, we
> + return an error */
> + if (start_revnum > head_revnum && end_revnum > head_revnum)
> + {
> + err = 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, head_revnum);
> + return err;
> + }
> +
> + if (start_revnum > head_revnum || end_revnum > head_revnum)
> + {
> + err = 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,

Odd formating, why the trailing backslash? Subversion usually puts
spaces round '?' and ':'.

> + head_revnum);
> + return err;
> + }
> +
> + if (start_revnum < 0 && end_revnum < 0)
> + {
> + err = 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);
> + return err;
> + }
> +
> + if (start_revnum < 0 || end_revnum < 0)
> + {
> + err = 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);
> + return err;
> + }

Later in this function there is code that handles a new repository
with no revisions:
a) there is now code duplication retrieving HEAD.
b) I think you broke it.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jul 29 20:27:39 2002

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.