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

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2004-09-28 04:44:02 CEST

Garrett Rooney wrote:
> Philip Martin wrote:
>> You risk having values greater than INT_MAX converted into negative
>> numbers (and the svn_repos_get_log3 documentation doesn't say what
>> happens if limit is negative :)
> True. Perhaps an unsigned int would be more appropriate? I can't think
> of any reasonable behavior for a negative limit.
>> Of course values greater than INT_MAX won't occur if you are starting
>> with an int in the first place... which raises a question about
>> ra_svn_log2: it's probably not valid to pass an int to
>> svn_ra_svn_write_tuple with an "n" format, as that will attempt to
>> retrieve 8 bytes from the varargs when only 4 bytes have been passed.

Ok, here's a proposed patch to correct the problem on both sides of the

This doesn't answer the 'signed vs unsigned' limit argument question
though, what do people think about that?


* subversion/libsvn_ra_svn/client.c
   (ra_svn_log2): convert limit argument into an apr_uint64_t
    before passing it in to svn_ra_svn_write_tuple.

* subversion/svnserve/serve.c
   (log_cmd): make limit an apr_uint64_t to avoid warnings about
    signed/unsigned comparison and guard against values larger
    than INT_MAX so we don't pass really big values into
    svn_repos_get_logs3. cast limit to int before passing into

Index: subversion/libsvn_ra_svn/client.c
--- subversion/libsvn_ra_svn/client.c (revision 11155)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -1034,7 +1034,7 @@
   SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(?r)(?r)bbn)", start, end,
                                  discover_changed_paths, strict_node_history,
- limit));
+ (apr_uint64_t) limit));
   SVN_ERR(handle_auth_request(sess, pool));
   /* Read the log messages. */
Index: subversion/svnserve/serve.c
--- subversion/svnserve/serve.c (revision 11155)
+++ subversion/svnserve/serve.c (working copy)
@@ -887,14 +887,19 @@
   apr_array_header_t *paths, *full_paths;
   svn_ra_svn_item_t *elt;
   int i;
- int limit;
+ apr_uint64_t limit;
   log_baton_t lb;
   /* Parse the arguments. */
   SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb?n", &paths,
                                  &start_rev, &end_rev, &changed_paths,
                                  &strict_node, &limit));
+ /* if we got an unspecified number then the user didn't send us anything,
+ so we assume no limit. if it's larger than INT_MAX then someone is
+ messing with us, since we know the svn client libraries will never send
+ us anything that big, so play it safe and default to no limit. */
+ if (limit == SVN_RA_SVN_UNSPECIFIED_NUMBER || limit > INT_MAX)
     limit = 0;
   full_paths = apr_array_make(pool, paths->nelts, sizeof(const char *));
@@ -912,9 +917,9 @@
   /* Get logs. (Can't report errors back to the client at this point.) */
   lb.fs_path = b->fs_path;
   lb.conn = conn;
- err = svn_repos_get_logs3(b->repos, full_paths, start_rev, end_rev, limit,
- changed_paths, strict_node, NULL, NULL,
- log_receiver, &lb, pool);
+ err = svn_repos_get_logs3(b->repos, full_paths, start_rev, end_rev,
+ (int) limit, changed_paths, strict_node,
+ NULL, NULL, log_receiver, &lb, pool);
   write_err = svn_ra_svn_write_word(conn, pool, "done");
   if (write_err)

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 28 04:43:53 2004

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.