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
network...
This doesn't answer the 'signed vs unsigned' limit argument question
though, what do people think about that?
-garrett
* 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
svn_repos_get_logs3.
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 (limit == SVN_RA_SVN_UNSPECIFIED_NUMBER)
+
+ /* 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